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.

Sign up for GitHub

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

Jump to bottom

Package.json does not properly identify its ESM dist as ESM #16

Closed
JakobJingleheimer opened this issue Nov 22, 2021 · 0 comments · Fixed by #17
Closed

Package.json does not properly identify its ESM dist as ESM #16

JakobJingleheimer opened this issue Nov 22, 2021 · 0 comments · Fixed by #17

Comments

@JakobJingleheimer
Copy link
Contributor

JakobJingleheimer commented Nov 22, 2021 •
edited
Loading

🐛 Bug Report

When react-merge-refs is imported by a project, its CJS & ESM distributions are not properly identified, causing only the CJS dist to be registered. react-merge-refs's package.json uses only the non-standard webpack "module" field.

To Reproduce

Steps to reproduce the behavior:

  1. Install react-merge-refs as a dependency of any ESM project
  2. import it in any file (ex test.mjs)
  3. run test.mjs with Node.js (node ./test.mjs)
  4. The CJS dist gets used instead of the ESM

Live repro

Expected behavior

The ESM distribution should get used.

I am happy to send a PR to fix this, if you could first confirm your compatibility support—specifically, do you care about very old versions of Node.js? (I don't see anything in react-merge-refs's configs that would suggest you do, and I see you're targeting ESNext, so I would think from that you don't). Supporting very old versions is possible but is significantly more complicated and would add complexity to the project (whereas not won't add much complexity at all).
@JakobJingleheimer JakobJingleheimer mentioned this issue Jan 4, 2022
Merged
kylegach added a commit to storybookjs/components-marketing that referenced this issue Aug 31, 2022
@kylegach
Fixes for integration
63f7a63 
- Downgrade `react-merge-refs` to `^1.10`
    - `^2.0.0` is ESM-only (https://github.com/gregberge/react-merge-refs/releases/tag/v2.0.0), making integration in various projects (namely Next.js) difficult at the moment
    - This may cause issues in environments like jest: gregberge/react-merge-refs#16
        - Currently not an issue, as there are no tests
- Remove `@docsearch/css` import from Search
    - Next.js requires all global CSS to be imported in the top-level App component
    - Add instructions to README
@kylegach kylegach mentioned this issue Aug 31, 2022
Merged
kylegach added a commit to storybookjs/components-marketing that referenced this issue Aug 31, 2022
@kylegach
Fixes for integration
778d242 
- Downgrade `react-merge-refs` to `^1.10`
    - `^2.0.0` is ESM-only (https://github.com/gregberge/react-merge-refs/releases/tag/v2.0.0), making integration in various projects (namely Next.js) difficult at the moment
    - This may cause issues in environments like jest: gregberge/react-merge-refs#16
        - Currently not an issue, as there are no tests
- Remove `@docsearch/css` import from Search
    - Next.js requires all global CSS to be imported in the top-level App component
    - Move it from deps to peerDeps
    - Add instructions to README
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix CJS & ESM distribution JakobJingleheimer/react-merge-refs
1 participant
@JakobJingleheimer