-
Notifications
You must be signed in to change notification settings - Fork 325
fix: add typings for alternative entrypoints #506
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
base: master
Are you sure you want to change the base?
Conversation
These were present in `@types/mixpanel-browser` before but got lost when inlining them
Hey thanks for the contribution! If I'm understanding correctly, the use case here is to be able to import the pre-built bundles like |
This is correct. It's currently preventing me (and likely others) from updating to the latest version of the mixpanel library. |
@@ -0,0 +1 @@ | |||
export * from '../src/index'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code isn't going to be copied into dist/
as part of the build process (your current branch will have this change overridden when running npm run build-dist
). I think the easiest way would be to add additional entrypoints in package.json like
Lines 27 to 31 in 012f1ee
"./src/loaders/loader-module-core": { | |
"types": "./src/index.d.ts", | |
"import": "./src/loaders/loader-module-core.js", | |
"require": "./src/loaders/loader-module-core.js" | |
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah. I opened this PR before you introduced the exports
in package.json
. I'll try to revisit this asap.
Actually these pre-built files aren't even accessible due to the introduction of exports
anymore atm, not only the typings are missing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can take this over if you can't get it to it, apologies for the trouble. Since we made these custom builds we said in our docs to import the non-built loaders (e.g. src/loaders/loader-module-core
) because I thought that would be preferable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would be great if you could handle it. I would close this PR then.
These were present in
@types/mixpanel-browser
before but got lost when inlining them. I originally introduced them in the external types through DefinitelyTyped/DefinitelyTyped#72165.Fixes #501