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: importing through mjs in webpack@4 #615

Closed
wants to merge 1 commit into from

Conversation

davidyuk
Copy link

@davidyuk davidyuk commented Feb 9, 2022

I have trouble importing this package in an mjs file in a project created using create-react-app@4.0.3.

Here is a reproduction: https://github.com/davidyuk/reproductions/tree/react-uuid

You need to run yarn && yarn build the see the error:

Failed to compile.

./src/test.mjs
Can't import the named export 'v4' from non EcmaScript module (only default export is available)

create-react-app@4.0.3 uses webpack@4 that resolves dependencies differently if mjs files are involved. It is fixable by enabling javascript/auto for mjs, but create-react-app doesn't allow to change configuration without ejecting. So, I'm proposing to apply a fix that makes it works, it is based on webpack/webpack#7482 (comment).

@ctavan
Copy link
Member

ctavan commented Aug 5, 2022

Is this still an issue?

@davidyuk
Copy link
Author

davidyuk commented Aug 9, 2022

Yep! There is no minor updates for "react-scripts" package. I've rechecked reproduction, it is still actual.

@broofa
Copy link
Member

broofa commented Aug 9, 2022

@davidyuk That's expected. Until react-scripts updates their dependencies this will continue to be an issue with that package. I believe you can patch the dependency, however, by adding the following to your package.json:

  "overrides": {
    "uuid": "9.0.0-beta.0"
  }

Then npm install and verify the new version of uuid is used by react-scripts:

$ npm ls uuid
foo@1.0.0 /private/tmp/foo
`-- react-scripts@5.0.1
  `-- webpack-dev-server@4.9.3
    `-- sockjs@0.3.24
      `-- uuid@9.0.0-beta.0

Give that a try and let us know if you still see the problem.

@davidyuk
Copy link
Author

Firstly I didn't noticed 9.0.0-beta.0 release, using this version in my reproduction didn't solves the issue, the error is same.

Until react-scripts updates their dependencies

The issue is not in dependencies of react-scripts but uuid can't be imported by name in an mjs user's module if it is build by react-scripts@4. It can be argued to use react-scripts@5 that works well because of webpack@5, but I beleive there is the same issue in react native because the react native scripts still using webpack@4.

I've rebased my fix on the top of main and published as @aeternity/uuid@0.0.2, my reproduction gets fixed using it.

@ctavan
Copy link
Member

ctavan commented Aug 12, 2022

Thanks for looking into this again.

I'm still a bit reluctant of adding support for a niche feature here (.mjs support), that was present in webpack@4 but that is no longer an issue in webpack@5 given proper pkg.exports support.

I'm leaning towards thinking that users who decide to use .mjs with webpack@4 which I would consider more of an intermediate standard that didn't age well should deal with this either by upgrading to webpack@5 or by following the solution described in https://stackoverflow.com/questions/64002604/how-to-make-create-react-app-support-mjs-files-with-webpack

Did you try the react-app-rewired approach?

@davidyuk
Copy link
Author

Did you try the react-app-rewired approach?

I know it would work. Our case is that we building a library wich decided to use .mjs and be compatible with react-script@4, also we depend on uuid, and want to simplify instalation as much as possible in multiple environments.

React native using own builder (not webpack@4) that currently doesn't support mjs out of the box, but this would be fixed in the next release: facebook/metro#535.

My concerns was primally about react native, but this is not related case and since uuid and mjs works fine in react-scripts@5, this fix is not so important for me. It was interesting to dig into all of this stuff 🙂

@broofa
Copy link
Member

broofa commented Oct 12, 2023

Closing this out for lack of activity and interest. Let me know if we should reopen.

This pull request was closed.
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.

3 participants