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 ES Module interop #35

Merged
merged 3 commits into from
Feb 10, 2020
Merged

Fix ES Module interop #35

merged 3 commits into from
Feb 10, 2020

Conversation

hofman-stripe
Copy link
Contributor

Summary & motivation

Fix #33

#31 used names exports for package "prop-types", which doesn't work when using this package as an ES Module.
Partially reverts #31 to use default imports for packages that expect them, but make sure generated types don't use default import to preserve the intent of #31.

Testing & documentation

Verified in a test project the exported types are complete.
Added a test on generated type definitions to ensure we don't re-introduce default import for types in the future.

Note

Check v1.0.0-beta.4...hofman/fix-esmodule-interop to see the combined change with #31

Copy link
Contributor

@christopher-stripe christopher-stripe left a comment

Choose a reason for hiding this comment

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

Seems like we'll break this in development frequently, but the test will prevent us from merging so I'm good with it. Left a minor suggestion.

package.json Outdated
@@ -14,9 +14,10 @@
"lint:prettier": "prettier './**/*.js' './**/*.css' './**/*.md' --list-different",
"typecheck": "tsc",
"build": "yarn run clean && yarn run rollup -c",
"checkimport": "grep 'import [^*{]' dist/react-stripe.d.ts; case $? in 1) true ;; 0) echo Found invalid import; false ;; *) false ;; esac",
Copy link
Contributor

Choose a reason for hiding this comment

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

(2) Neat. Would be nice to read this in a separate file in a scripts directory, with a comment on how to fix this issue

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

rollup.config.js Show resolved Hide resolved
Copy link
Contributor

@christopher-stripe christopher-stripe left a comment

Choose a reason for hiding this comment

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

Awesome, ty!

@hofman-stripe hofman-stripe merged commit 3cfda01 into master Feb 10, 2020
@danseaman6
Copy link

Thanks guys! Can you republish the NPM module?

@hofman-stripe
Copy link
Contributor Author

Published as v1.0.0-beta.6!

@dweedon-stripe dweedon-stripe deleted the hofman/fix-esmodule-interop branch February 12, 2020 12:21
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.

ElementsConsumer.propTypes - func undefined
3 participants