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

fix: enable eslint and follow it's errors #28

Merged
merged 3 commits into from
Feb 1, 2024

Conversation

auto200
Copy link
Contributor

@auto200 auto200 commented Jan 31, 2024

  • set up formatting scripts but did not format whole project yet since there would be too much noise in the diff for this PR

  • eslint config was broken and did not report errors, used minimal https://typescript-eslint.io/ setup since core package does not use react (yet)
    image

  • created proper types to replace anys

  • CustomEvent.trackEvent now properly accepts dimensions as a parameter, updated docs and example

caveat

when bundling with microbundle-crl type information about Dimensions type gets lost

type Dimensions = Record<`dimension${number}`, string>

dist/interfaces/utils.d.ts
image

resulting with any type

image

confirmed with regular tsc build and it gets compiled properly. We could consider switching microbundle-crl to something else

@auto200 auto200 marked this pull request as ready for review February 1, 2024 12:16
@lysy-vlc
Copy link
Contributor

lysy-vlc commented Feb 1, 2024

How about using vite? We already have conf files from vue package

@auto200
Copy link
Contributor Author

auto200 commented Feb 1, 2024

How about using vite? We already have conf files from vue package

That's a solid option to consider, can we postpone it until we extract core package? Definitely in the separate PR, not to do everything at once

@auto200 auto200 merged commit 1fb84dd into master Feb 1, 2024
4 checks passed
@auto200 auto200 deleted the fix/eslint-and-proper-types branch February 1, 2024 13:37
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