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

Does core require a new babel loader? #4112

Closed
pudo opened this issue May 5, 2020 · 13 comments
Closed

Does core require a new babel loader? #4112

pudo opened this issue May 5, 2020 · 13 comments

Comments

@pudo
Copy link

pudo commented May 5, 2020

Environment

  • Package version(s): 3.26.1

If possible, link to a minimal repro (fork this code sandbox): https://github.com/alephdata/aleph/blob/master/ui/package.json

Steps to reproduce

  1. Install @blueprint/core inside a create-react-app app.
  2. Attempt to compile

Actual behavior

./node_modules/@blueprintjs/core/lib/esnext/components/forms/numericInput.js 60:31
Module parse failed: Unexpected token (60:31)
File was processed with these loaders:
 * ./node_modules/babel-loader/lib/index.js
You may need an additional loader to handle the result of these loaders.
|       const delta = this.updateDelta(direction, e);
|       const nextValue = this.incrementValue(delta);
>       this.props.onButtonClick?.(+nextValue, nextValue);
|     };
|

Possible solution

Not clear on what further configuration is required.

@adidahiya
Copy link
Contributor

It does if your build system picks up the esnext module entry point specified in package.json:

"esnext": "lib/esnext/index.js",

This build target was added in #3230... but now I'm looking at the tsconfig options documentation and --target esnext appears to be discouraged since it is a moving target. I wonder if we should stick to a more predictable --target es2019 instead. Or maybe CRA should be favoring the "module" field over "esnext"? The latter is less standardized...

Sorry for the unintentional break @pudo

@maclockard @ericanderson I wonder if you have any insight here?

@maclockard
Copy link
Contributor

@pudo this is happening since you are directly importing an esnext file here: https://github.com/alephdata/aleph/blob/7ab658998ae41aa86ee15bf93f3f4f6911235ba6/ui/src/components/Screen/Screen.jsx#L5-L6. While this fixes the es5 vs es6+ class compatibility issue, its now causing a problem since the create-react-app's babel-loader can't handle the optional chain operator (?.) which blueprint makes use of. There's a couple ways to fix this I think.

An easy and quick way would be to upgrade to a new version of babel, since they added support for both optional chaining and nullish coalescing in 7.8.0 (CHANGELOG). I'm not too sure about your setup or create-react-app's setup, but upgrading to a newer version of create-react-app should fix this, looks like newer versions are on babel 7.9.6 (link) which should cover this. If upgrading create-react-app is not an option, you can maybe try to force resolve the transitive dependency in your repo. Not sure how to do this with npm, but yarn offers some tooling here.

An option with slightly longer turn around would be for blueprint to add another target like es2018 or es2019, but I don't see too much of an advantage here since most folks using esnext should be handling babeling their dependencies themselves since its more of an experimental evergreen thing than something explicitly supported. A more sustainable solution to prevent a need for targeting every es version would be to add alternatives to the decorator for both hot keys and context menus.

@ChristianIvicevic
Copy link
Contributor

Just wanted to chime in and say that I have the very same issue and most likely since I am importing the HotkeysTarget similar to OP since it wouldn't work otherwise without specified the exact esnext import path. A quick resolution on this issue would be highly appreciated, in the meantime I have to go back to a pinned older version that still works.

@maclockard
Copy link
Contributor

@ChristianIvicevic are you unable to upgrade babel versions? that's the quick resolution outlined above. You will also need to make sure you babel blueprint as a part of your build process.

@ChristianIvicevic
Copy link
Contributor

@maclockard Thank you for your fast reply. I am currently running react-scripts 3.4.1 which uses babel 7.9.0. I assume that upgrading other dependencies caused the lockfile to be partially regenerated since everything worked with blueprint 3.25.0 a few moments ago.

So I am already running the latest published version of react-scripts with the respective babel version. Since I haven't ejected I am not in control of the. underlying babel configuration and versions and would prefer not to mess around with it too much and I don't actually know how to enforce a newer babel version to used right now.

Out of curiousity I tried removing the HotkeysTarget reference entirely and get the same error, so it's "just" a compilation error when attempting to import @blueprint/core.

@maclockard
Copy link
Contributor

can you copy and paste your compilation error? I would expect that version of babel to be able to properly handle an optional chain operator.

@ChristianIvicevic
Copy link
Contributor

It's virtually the same that OP had.

Failed to compile.

./node_modules/@blueprintjs/core/lib/esnext/components/forms/numericInput.js 60:31
Module parse failed: Unexpected token (60:31)
File was processed with these loaders:
 * ./node_modules/react-scripts/node_modules/babel-loader/lib/index.js
You may need an additional loader to handle the result of these loaders.
|       const delta = this.updateDelta(direction, e);
|       const nextValue = this.incrementValue(delta);
>       this.props.onButtonClick?.(+nextValue, nextValue);
|     };
|

@ChristianIvicevic
Copy link
Contributor

I found a bizzare fix! It is described in the following PR: facebook/create-react-app#8526

The browserlists in package.json is per default super strict with last 1 chrome version. However it seems that there is a mismatch with the newest Chrome version 80 that supports certain nullish features. When I replaced last 1 chrome version with last 10 chrome version it suddenly worked and compiled correctly. I still have to try setting the value to chrome 79 to see what happens then.

@maclockard
Copy link
Contributor

ah interesting, that makes sense

@ChristianIvicevic
Copy link
Contributor

ChristianIvicevic commented May 16, 2020

Quick update, adding chrome 79 in addition to last 1 chrome version fixed the problem. I have to be honest, since I never worked with browserlists myself and took this feature for granted, I am not entirely sure why it fixed the problem, at least it is a quick fix for the time being until the underlying acorn issue described in the original PR is resolved.

(Nevertheless I'd appreciate that HotkeysTarget is made usable at some point in the future without importing the esnext module manually since it has come up during this issue, even though it wasn't the culprit.)

pudo added a commit to alephdata/aleph that referenced this issue May 21, 2020
@pudo
Copy link
Author

pudo commented May 21, 2020

Thanks for the comprehensive feedback on this. The work-around of specifying chrome 79 in the browsers list works, so as an immediate issue this is gone for us. We could close the issue or maybe there's a more long-term code change you wanna base off this?

@JLHwung
Copy link

JLHwung commented Oct 22, 2020

The error is thrown from acorn 6 in webpack 4, it is fixed in webpack 5: webpack/webpack#11186

Unfortunately due to breaking changes from acorn 6.4.0 to 7.0.0, webpack 4 can not ship acorn 8, which supports optional chaining.

Specifying chrome 79 can work around this issue because @babel/preset-env will apply optional chaining for chrome 79. Since chrome 80 supports optional chaining, it will not transpile optional chaining and thus acorn throw parsing error.

Alternatively, you can also wait until CRA updates to webpack 5: facebook/create-react-app#7929, when webpack 5 is used, please remove chrome 79 so @babel/preset-env can skip most transpling.

dlech added a commit to pybricks/pybricks-code that referenced this issue Nov 21, 2020
- had to update eslint deps
- ran into palantir/blueprint#4112
- had to move beta.svg out of static/
- had to fix prettier formatting changes
- ran into jestjs/jest#7780
dlech added a commit to pybricks/pybricks-code that referenced this issue Nov 21, 2020
- had to update eslint deps
- ran into palantir/blueprint#4112
- had to move beta.svg out of static/
- had to fix prettier formatting changes
- ran into jestjs/jest#7780
dlech added a commit to pybricks/pybricks-code that referenced this issue Nov 21, 2020
- had to update eslint deps
- ran into palantir/blueprint#4112
- had to move beta.svg out of static/
- had to fix prettier formatting changes
- ran into jestjs/jest#7780
dlech added a commit to pybricks/pybricks-code that referenced this issue Nov 21, 2020
- had to update eslint deps
- ran into palantir/blueprint#4112
- had to move beta.svg out of static/
- had to fix prettier formatting changes
- ran into jestjs/jest#7780
@adidahiya
Copy link
Contributor

I don't think there's any more action on Blueprint's part required here. By definition, the esnext target will produce JS in lib/esnext which often requires post-processing to run in a common JS environment. If your build system picks up this entry point, it's your responsibility to do that post-processing.

Also, re: the last comment in this thread, Webpack 5 has been out for some time now and we have now upgraded to it in this repo without any issues.

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

No branches or pull requests

5 participants