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

upgrading to react-script 3.3.1 breaks optional chaining #8445

Closed
ggascoigne opened this issue Feb 9, 2020 · 17 comments · Fixed by #8526
Closed

upgrading to react-script 3.3.1 breaks optional chaining #8445

ggascoigne opened this issue Feb 9, 2020 · 17 comments · Fixed by #8526
Milestone

Comments

@ggascoigne
Copy link

Since upgrading to react-scripts 3.3.1 my code, which uses optional chaining, fails to compile.

Module parse failed: Unexpected token (22:12)
File was processed with these loaders:
 * ./node_modules/babel-loader/lib/index.js
 * ./node_modules/eslint-loader/dist/cjs.js
You may need an additional loader to handle the result of these loaders.
|   // library barfs if you have an undefined value here instead of an empty array
|
>   if (props?.SelectProps?.multiple && field.value === undefined) {
|     field.value = [];
@lnhrdt
Copy link

lnhrdt commented Feb 9, 2020

@ggascoigne I encountered the same problem when upgrading to 3.3.1. I was able to resolve the issue and stay on 3.3.1 by deleting my node_modules folder and reinstalling my dependencies.

I don't understand the underlying issue but sharing this info in case it helps others.

@JeromeDeLeon
Copy link
Contributor

JeromeDeLeon commented Feb 9, 2020

Removing node_modules didn't work in my case or perhaps this is not related to my question regarding calling function with ?. in it, like so, getSum?.()?

ESLint complains about Expected an assignment or function call and instead saw an expression. eslint(no-unused-expressions)

PS: Disabling eslint in my VSCode worked.

@yilinjuang
Copy link

@lnhrdt removing node_modules works for me. Thanks.

@lvpro
Copy link

lvpro commented Feb 10, 2020

Same issue here. Removing node_modules did not help.

We're ejected, though, so I added @babel/plugin-proposal-nullish-coalescing-operator to our .babelrc file myself, then it started working again.

@PatrickZGW
Copy link

Removing node_modules did not work for me. Our project is not ejected.

@hanneswidrig
Copy link

This issue is still present on 3.4.0.

@justingrant
Copy link
Contributor

Looking at the diff of 3.3.0 vs. 3.3.1, I'm guessing that #8353 (FYI @ianschmitz) may be the culprit because it removed the babel plugins for optional chaining, null coalescing, and dynamic import.

To verify that this was the culprit, I manually added the optional chaining and null-coalescing babel plugins to CRA 3.4.0 using customize-cra. When I did this, npm start no longer reported any errors from use of optional-chaining and null-coalescing operators.

Until this is fixed, here's workaround instructions that worked for me for those two operators:

  1. Follow the instructions in https://medium.com/@adostes/enabling-optional-chaining-in-a-create-react-app-a9f626a515d9 to install customize-cra and configure it to add optional chaining via a custom .babelrc. This step should get optional chaining working. The next steps will get null-coalescing working too.
  2. npm i -D @babel/plugin-proposal-nullish-coalescing-operator
  3. Add null-coalescing plu to .babelrc. You should end up with a .babelrc that looks like this:
// .babelrc
{
  "plugins": [
    "@babel/plugin-proposal-optional-chaining",
    "@babel/plugin-proposal-nullish-coalescing-operator"
  ]
}

FYI: I couldn't get customize-cra's AddBabelPlugin method to work, but useBabelRc() seems to work fine so I didn't spend time to figure out what's going wrong with AddBabelPlugin().

@ianschmitz
Copy link
Contributor

ianschmitz commented Feb 19, 2020

@justingrant would you be able to upload your lock file here by chance?

It's tricky to debug as a fresh app doesn't reproduce:
image

@justingrant
Copy link
Contributor

@ianschmitz - Sure, here's my package-lock.json: https://gist.github.com/justingrant/cb9b3c088f90abd8382141ebe608f141

In case it's helpful, here's my package.json too (with private stuff removed): https://gist.github.com/justingrant/194b21ccea3510ee8ef972c0cd9d8ca4

FWIW, I tried rebuilding my node_modules and that didn't fix the problem. I get the same problem on 3.3.1 and 3.4.0.

Also, while researching this problem I found a similar issue in the webpack repo: webpack/webpack#10227. I'm not sure if this is the same root cause because that issue uses ts-loader for typescript but my app uses babel. But at least wanted to capture it here to save others some Googling in case it's related. That repro seems to depend on target: "ESNext", which made me wonder if it could be related to my dev-mode browserslist config in package.json:

  "browserslist": {
    "production": [
      ">0.2%",
      "not dead",
      "not op_mini all"
    ],
    "development": [
      "last 1 chrome version"
    ]
  }

Here's two other issues which I came across which also might be (or might not be!) related to this issue:

@ianschmitz
Copy link
Contributor

ianschmitz commented Feb 19, 2020

Interesting! Can you try updating your browserslist dev settings to include ie 11 as a test and see if the issue persists? You may have to clear out node_modules after doing so as sometimes the babel cache can persist (at least in past this happened).

@ianschmitz
Copy link
Contributor

I think i've tracked down the source of the issue. It looks like we will require acornjs/acorn#890 and acornjs/acorn#891 to be merged and included by webpack before we can fully delegate the decision to transform optional chaining and nullish coalescing to browserslist completely. This syntax isn't yet understood by acorn which webpack uses to parse the source. I'll test this more later to confirm when i get the time.

Assuming the above is the case, we will add the plugins back in the next patch release until we get a newer version of webpack that will support the syntax. Until then a quick fix could be to update your browserslist config to include chrome 79 if you were using an aggressive list like the last 1 chrome version example above, as it sounds like support was added in version 80: https://caniuse.com/#search=optional%20chaining.

@justingrant
Copy link
Contributor

@ianschmitz - Sorry for delayed response. Looks like you already identified the root cause. Do you still need me to test ie 11? Your proposed diagnosis and solution sounds right, BTW. I did confirm that changing browserslist without clearing node_modules does NOT solve the problem, at least in my app.

Also, where does the babel cache live? Can I just delete it, instead of all of node_modules? I'd love to avoid re-creating node_modules every time I want to change my browserslist!

@ianschmitz
Copy link
Contributor

ianschmitz commented Feb 19, 2020

It should live at node_modules/.cache. Hopefully that's still the case. We've documented it in our docs to help out: https://create-react-app.dev/docs/supported-browsers-features/#configuring-supported-browsers.

I haven't had a chance to verify this in-depth, so would appreciate to get additional verification from you folks before we restore the plugins. Thanks! 😄

@yordis
Copy link

yordis commented Feb 19, 2020

Confirmed in our end that removing node_modules because of the Babel cache works (we are not using CRA but we are using the babel preset)

@justingrant
Copy link
Contributor

@ianschmitz - I can confirm that adding chrome 79 to browserslist causes the problem to go away after deleting node_modules/.cache, quitting the dev server, and restarting it with npm start.

@JeromeDeLeon
Copy link
Contributor

I can confirm it. Same as @justingrant did

@ianschmitz
Copy link
Contributor

Perfect. Thanks everyone. Will have a PR fix up shortly.

@ianschmitz ianschmitz added this to the 3.4.1 milestone Feb 20, 2020
@lock lock bot locked and limited conversation to collaborators Feb 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants