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

Nullish coalescing operator makes package incompatible with webpack 4 #883

Closed
niklasnatter opened this issue May 18, 2021 · 15 comments
Closed

Comments

@niklasnatter
Copy link

niklasnatter commented May 18, 2021

Hey,
first of all - thanks a lot for working on this library, we really enjoy using it!

Unfortunately, the recently released version 3.2.0 of this package breaks our build. I am aware that this was already reported in #881, #882, #879, #877 and #880, but I would like to add some additional information here why this is happening and what the possible solutions are. For completion, the build in our project errors with the following message:

ERROR in /sulu-skeleton/vendor/sulu/sulu/node_modules/react-leaflet/esm/Pane.js 25:37
Module parse failed: Unexpected token (25:37)
You may need an appropriate loader to handle this file type, currently no loaders are configured to process this file. See https://webpack.js.org/concepts#loaders
|   }
| 
>   const parentPaneName = props.pane ?? context.pane;
|   const parentPane = parentPaneName ? context.map.getPane(parentPaneName) : undefined;
|   const element = context.map.createPane(name, parentPane);
 @ /sulu-skeleton/vendor/sulu/sulu/node_modules/react-leaflet/esm/index.js 13:0-30 13:0-30
 @ /sulu-skeleton/vendor/sulu/sulu/src/Sulu/Bundle/LocationBundle/Resources/js/containers/Location/Location.js
 @ /sulu-skeleton/vendor/sulu/sulu/src/Sulu/Bundle/LocationBundle/Resources/js/containers/Location/index.js
 @ /sulu-skeleton/vendor/sulu/sulu/src/Sulu/Bundle/LocationBundle/Resources/js/containers/Form/fields/Location.js
 @ /sulu-skeleton/vendor/sulu/sulu/src/Sulu/Bundle/LocationBundle/Resources/js/containers/Form/index.js
 @ /sulu-skeleton/vendor/sulu/sulu/src/Sulu/Bundle/LocationBundle/Resources/js/index.js
 @ ./index.js

Why is it happening

Version 3.2.0 of this package uses the nullish coalescing operator in the code that is published to npm. As this operator is supported by most modern browsers, babel does not transpile it away when defining a fairly modern set of browsers in the browserlist used by the project.

Unfortunately, while supported by modern browsers, the nullish coalescing operator is not supported by webpack 4 (webpack/webpack#10227). The reason for this is that support for the operator was only added in acorn 7 (acornjs/acorn#890), but webpack 4 uses (and probably will always use) acorn 6.

In conclusion, this means that this package is currently not compatible with webpack 4 without additional configuration because it includes the nullish coalescing operator in the code that is published to npm. Webpack 4 is still used by popular projects such as create-react-app.

How to fix it in the library

The most effective solution for this problem would be publishing a new version of the package that does not include the nullish coalescing operator. This is either possible by adjusting the code of the package like proposed in #880 or by adjusting the .babelrc.js file in this repository transpile this operator in the code that is published to npm.

This solution would allow all projects that are encountering the problem to fix it by simply updating the package.

How to fix it in your project

Its possible to prevent the problem in your project by configuring babel to always transpile the nullish coalescing operator regardless of the targeted browsers by enabling the @babel/plugin-proposal-nullish-coalescing-operator plugin: https://babeljs.io/docs/en/babel-plugin-proposal-nullish-coalescing-operator

This plugin will transpile the nullish coalescing operator before the files are processed by webpack and therefore prevent the error in your project.

Other workarounds

A lot of comments suggest to adjust the targeted browsers to include older browsers like described in facebook/create-react-app#9468 (comment)
This fix will indirectly enable the @babel/plugin-proposal-nullish-coalescing-operator plugin. The disadvantage of this method is that the size of the build will increase because the whole codebase is transpiled to be fully compatible with older browsers.

Other comments like #879 (comment) suggest to skip the 3.2.0 version of the package. This is only a temporary fix and things will break again if the next version of the package still contains the nullish coalescing operator in the published code.

@GianfrancoCorrea
Copy link

I'm having the same issue, but i have the config on babel to handle nullish coalescing operator from months ago. and doesn't work for this library.
its seems like I need a transpile for react-leaflet in my next.config.js/webpack.config.js 🤔

@monsonjeremy
Copy link

monsonjeremy commented May 18, 2021

Having the same issue here, it's causing me to have to add quite a few workarounds to my configurations. I think it's negligible in terms of bundle size to just compile this down. I can take a look tonight and submit a PR.

Edit: Nevermind, it looks like @PaulLeCam has rejected a PR for this before.

@PaulLeCam it is not standard practice for packages to have the latest ES syntax because bundlers like webpack are configured by default to not run babel on node_modules (I mean imagine running babel on all 1000+ packages every time you build). The fix should be as simple as changing the output targets of the build command. If you can confirm that you're okay with this then I'll be happy to take a look.

@jschill
Copy link

jschill commented May 18, 2021

I can confirm this with FuseBox 3 as well (It's using Acorn 5.7).

@monsonjeremy
Copy link

Also this doesn't exist only with Webpack. Babel on Node 12 can have the issue in certain cases. I have some code that dynamically imports react leaflet (since I need to server-side render) and when I run my tests in node < 14 it's not able to run.

@nbaleli-w
Copy link

nbaleli-w commented May 19, 2021

reverting to 3.1.0 doesn't seem to help.
explicitly installing @react-leaflet/core@1.0.2 does solve the issue

@monsonjeremy
Copy link

I've opened a PR here: #885

@ilkerbfl
Copy link

None of the comments above worked for me. Changing versions in package.json solved issue.

"react-leaflet": ">=3.1.0 <3.2.0 || ^3.2.1",
"@react-leaflet/core": ">=1.0.0 <1.1.0 || ^1.1.1",

use npm install instead of yarn install.
I hope this helps

@niklasnatter
Copy link
Author

None of the comments above worked for me. Changing versions in package.json solved issue.

"react-leaflet": ">=3.1.0 <3.2.0 || ^3.2.1",
"@react-leaflet/core": ">=1.0.0 <1.1.0 || ^1.1.1",

use npm install instead of yarn install.
I hope this helps

Be aware that this is not a solution. It is a workaround that prevents installing the 3.2.0 version of the package. The same error will pop up as soon as a new version of the package is released.

@nicolo-danzi
Copy link

nicolo-danzi commented May 20, 2021

As a temporary workaround you can specify "@react-leaflet/core": "1.0.2" as dependency and specify also
"resolutions": { "react-leaflet/@react-leaflet/core": "1.0.2" }
in package.json. This forces react-leaflet package to have @react-leaflet/core version 1.0.2 as dependency

@marcin-piechaczek
Copy link

For next.js project you can use

  future: {
    webpack5: true
  },

flag inside your next.config.js file

@sarkstephan
Copy link

sarkstephan commented Jun 2, 2021

Need to BABELify both packages

customize-cra (config-overrides.js)
...
const {
babelInclude,
addWebpackModuleRule
} = require('customize-cra');
...

babelInclude([
path.resolve(__dirname, 'src'),
path.resolve(__dirname, 'node_modules/react-leaflet'),
path.resolve(__dirname, 'node_modules/@react-leaflet'),
]),

for images
...
addWebpackModuleRule({ test: /.(png|jpg)$/, use: 'file-loader?name=images/[name].[ext]' }),

@morganney
Copy link

@PaulLeCam why was this closed?

@dennisat
Copy link

dennisat commented Nov 3, 2021

Still have this issue with v3.2.2

Real blocker for us, please fix this!!!!

Debugging this looks like the dist code is not distributed properly.

The building code of this repo is transpiring the nullish operator, but in npm's package it is still there.

@ulken
Copy link

ulken commented Nov 15, 2021

@PaulLeCam why was this closed?

See #885 (comment)

@ulken
Copy link

ulken commented Nov 15, 2021

@PaulLeCam while I don't agree with your take on this, would it not save you more work by adding this to the README/docs/wiki at least? Feels like this is coming up over and over again and no one understands why.

ellertsmari added a commit to ellertsmari/react-leaflet that referenced this issue Nov 17, 2021
Many of my students are having problems using react-leaflet because of issues reported in PaulLeCam#877,  PaulLeCam#880 and PaulLeCam#883. It seems to be a common problem: https://stackoverflow.com/questions/67552020/how-to-fix-error-failed-to-compile-node-modules-react-leaflet-core-esm-pat and there is even a video about the solution in YouTube with more than 1500 views: https://youtu.be/tFqj-JKYr4M
Merkur39 added a commit to cozy/coachCO2 that referenced this issue Sep 26, 2022
In anticipation of the v18 update of the react package.

The addition of a specific configuration is required with
webpack v4 since version 3 of react-leaflet.
See: PaulLeCam/react-leaflet#883
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet