Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve types for query #36

Merged
merged 2 commits into from
Feb 5, 2024
Merged

Conversation

calirvine
Copy link
Contributor

Reimplements the types from #26 with some small improvements, and the runtime checks removed.

);

return returnPromise;
}

async getVisitorCountByColumn(
async getVisitorCountByColumn<T extends keyof typeof ColumnMappings>(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the only real change (other than the runtime checks) from the previous PR. Introducing a generic here means that the return type can be inferred based on which column is passed.

@calirvine
Copy link
Contributor Author

@benvinegar I'm still seeing some prettier changes, I suspect it is because our editors are each picking up a different version. I think adding Prettier as a direct dev dependency should ensure all contributors are using the same version.

Copy link

codecov bot commented Feb 5, 2024

Codecov Report

Attention: 6 lines in your changes are missing coverage. Please review.

Comparison is base (2265b13) 72.38% compared to head (d5467ed) 72.75%.

Files Patch % Lines
app/analytics/query.ts 94.05% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #36      +/-   ##
==========================================
+ Coverage   72.38%   72.75%   +0.36%     
==========================================
  Files          18       18              
  Lines        1934     1960      +26     
  Branches      115      115              
==========================================
+ Hits         1400     1426      +26     
  Misses        528      528              
  Partials        6        6              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@benvinegar
Copy link
Owner

Even without the invariant stuff, this is a great improvement. Thanks!

@benvinegar benvinegar merged commit 71b54e3 into benvinegar:main Feb 5, 2024
1 check passed
benvinegar added a commit that referenced this pull request Dec 20, 2024
Co-authored-by: Cal Irvine <[email protected]>
Co-authored-by: Ben Vinegar <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants