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

Remove unnecessary 'browser' exports to fix Jest integration #435

Merged

Conversation

david-bezero
Copy link
Contributor

Fixes #396

This library is exporting multiple builds (umd/module/cjs) but duplicates the CJS ("require") build as the "browser" build. This is not required for this project; "browser" is supposed to be used for distinguishing browser-specific versions (e.g. if using Node crypto package when built for NodeJS vs. using browser-standard cryptography when built for browsers), whereas this project does not use any node-specific or browser-specific builds.

Jest's latest update reworks their import handling to properly support package.json exports, which has revealed this issue (Jest is now — correctly — using the browser import, but failing because it cannot handle esm modules)

By removing the browser entries entirely, all build systems will pick the appropriate build based on whether they want umd, import, or require. This seems to be in line with the original intent of the definitions.

Note also that with this change, the config/node-13-exports.js build config could also be changed to move the files instead of copying them, as nothing will require the old .module.js versions any more (unless users are importing them directly). I have left this change out to keep things minimal and fully backwards compatible.

Copy link
Member

@bluebill1049 bluebill1049 left a comment

Choose a reason for hiding this comment

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

Thanks for the fix! @jorisre is the master mind behind the setup, feel free to merge it when you think it's ok 👍

@shahmirn
Copy link

Could this also be ported back to the 1.x branch for people that are still using react-hook-form 6? Thanks!

@bluebill1049
Copy link
Member

I will merge this and release this first 👍

@bluebill1049 bluebill1049 merged commit 99703e6 into react-hook-form:master Jul 26, 2022
@github-actions
Copy link
Contributor

🎉 This PR is included in version 2.9.7 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Jest 28][Yup Resolver] SyntaxError: Cannot use import statement outside a module
3 participants