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

ES Numeric Separators broken by babel (Unexpected token: name (_000)) #645

Closed
westphalen opened this issue Mar 12, 2021 · 25 comments
Closed

Comments

@westphalen
Copy link

Do you want to request a feature or report a bug?
Bug / improvement for metro-react-native-babel-preset

What is the current behavior?

Error: Unexpected token: name (_000) in file App.tsx at 16:25

A build error that makes little sense in the context of a larger code base. The line:col isn't true to the original code, I had to console.log from within /metro/src/JSTransformer/worker.js to find the problematic line.

If the current behavior is a bug, please provide the steps to reproduce and a minimal repository on GitHub that we can yarn install and yarn test.

  • react-native init MyApp --template react-native-template-typescript
  • Use ES Numeric Separators syntax in your code, e.g. const thousand = 1_000;
  • Try to bundle the project for release

Minimal repository reproducing the issue:

  • git clone https://github.com/westphalen/metro-babel-preset-numeric-separators
  • yarn install
  • yarn build

What is the expected behavior?
Since the boilerplate project is set up to support TypeScript and es2017/esnext, one would expect that ES Numeric Separators were supported out of the box.

Solution
yarn install @babel/plugin-proposal-numeric-separator

Update babel.config.js with this line:

  plugins: ['@babel/plugin-proposal-numeric-separator'],

Would suggest this plugin is included by default in metro-react-native-babel-preset

Please provide your exact Metro configuration and mention your Metro, node, yarn/npm version and operating system.

  • macOS 10.15.7
  • node -v v10.22.0
  • "metro-react-native-babel-preset": "0.63.0"
  • metro "0.58.0"
@motiz88
Copy link
Contributor

motiz88 commented Mar 25, 2021

Hey @westphalen, would you be interested in sending a PR that adds @babel/plugin-proposal-numeric-separator to the preset?

@SConaway
Copy link
Contributor

@motiz88 after having a weird issue where numeric separators were only an issue when building for iOS without Hermes (it works fine in dev on iOS and on Android in dev and prod), i'd be willing to make a super-quick PR to do that. Would it be accepted?

Should it go in https://github.com/facebook/metro/blob/master/packages/metro-react-native-babel-preset/src/passthrough-syntax-plugins.js or https://github.com/facebook/metro/blob/master/packages/metro-react-native-babel-preset/src/configs/main.js?

@motiz88
Copy link
Contributor

motiz88 commented Jul 1, 2021

@SConaway I'd happily review a PR that adds the numeric separator plugin to https://github.com/facebook/metro/blob/master/packages/metro-react-native-babel-preset/src/configs/main.js. We need to transform this syntax rather than just pass it through, because some of the engines we target don't natively support it. I guess that's what you may have seen on iOS, since JavaScriptCore only added support for this syntax in iOS 13 per https://caniuse.com/mdn-javascript_grammar_numeric_separators.

@SConaway
Copy link
Contributor

SConaway commented Jul 4, 2021

Ok cool! Working on getting my developer environment setup now.

Is there are a reason why @babel/preset-env isn't used as a base plugin since it includes many of the ones that are currently in that file?

@motiz88
Copy link
Contributor

motiz88 commented Jul 6, 2021

@babel/preset-env differs in subtle ways from Metro's React Native preset. The latter accepts some different config options that are React Native-specific / Metro-specific; the former doesn't have a way to target Hermes specially (which is an experimental feature in Metro). Migrating to preset-env would be doable and probably beneficial to the ecosystem, but it's not trivial IMO.

@SConaway
Copy link
Contributor

SConaway commented Jul 6, 2021

Yes, fair. Let me know if there's anything else needed. I did update the list of included syntax transforms.

@cristianoccazinsp
Copy link

Any work around for libraries that use this syntax and need to be transformed in react-native?

@SConaway
Copy link
Contributor

Nope, my PR (#681) hasn’t been merged yet.

@hm-harshit
Copy link

is there any update on this issue ?

@FelipeSSantos1
Copy link

on metro.config.js I just included the babel plugin and it worked for me

module.exports = {
  transformer: {
    getTransformOptions: async () => ({
      transform: {
        experimentalImportSupport: false,
        inlineRequires: true,
      },
    }),
    plugins: ['@babel/plugin-proposal-numeric-separator'],
  },
}

@geraintwhite
Copy link

geraintwhite commented Nov 29, 2021

@felipemillhouse this works for me in debug mode, but not when building a release build from Xcode. Have you experienced this?

Error: Unexpected token: name (_28) in file node_modules/color/index.js at 240:30
    at minifyCode (/Users/jenny/workspace/Cloud_Voice_App_update-color/node_modules/metro-transform-worker/src/index.js:99:13)
    at transformJS (/Users/jenny/workspace/Cloud_Voice_App_update-color/node_modules/metro-transform-worker/src/index.js:317:28)
    at transformJSWithBabel (/Users/jenny/workspace/Cloud_Voice_App_update-color/node_modules/metro-transform-worker/src/index.js:408:16)
    at processTicksAndRejections (internal/process/task_queues.js:97:5)
    at Object.transform (/Users/jenny/workspace/Cloud_Voice_App_update-color/node_modules/metro-transform-worker/src/index.js:569:12)
info Run CLI with --verbose flag for more details.

Edit:

It seems like this issue is caused by metro-minify-uglify using uglify-es which has been deprecated in favour of uglify-js.

#661

@FelipeSSantos1
Copy link

FelipeSSantos1 commented Nov 29, 2021

@grit96 For me it also works on release (archiving). I'm using "react-native": "0.66.1" and "color": "^4.0.1" BTW.

Don't forget to yarn add @babel/plugin-proposal-numeric-separator -D

@geraintwhite
Copy link

Very strange. I'm on "react-native": "0.66.3" and "color": "4.0.2". Works perfectly archiving once I add a custom minifier copied from here but replacing uglify-es with uglify-js.

minifierPath: require.resolve('./config/minifier')

@mfbx9da4
Copy link
Contributor

For those wondering what may have caused this check your code for a number like 10_000 using the _ to separate numbers

@cristianoccazinsp
Copy link

Any thoughts on supporting this with metro? color maintainer refuses to do a simple "fix" to remove the underscore from numerical values.

@Dean177
Copy link

Dean177 commented Feb 4, 2022

Just switch to https://github.com/omgovich/colord
similar api, better performance, smaller size

@macrozone
Copy link

Just switch to https://github.com/omgovich/colord similar api, better performance, smaller size

thanks!

In my case i just needed to convert hex to rgb, i used https://github.com/sindresorhus/hex-rgb for that, but its actually a quite simple functions.

By the way, I was really puzzled about the attitude of the maintainer of color. I understand the decision to leaverage modern js, but locking all conversations without providing help is really anti-community. Not even others could give some hints, because he just locked all conversations about this topic. The issue tracker is filled with locked issues about that topic. And all of that because of 1 character in the code (which is otherwise not modern at all)...

@cristianoccazinsp
Copy link

The maintainer is kind of stubborn, but hey, anyone is free to fork this and fix that 1 char issue.

@macrozone
Copy link

The maintainer is kind of stubborn, but hey, anyone is free to fork this and fix that 1 char issue.

actually a lot of people did ;-)

It must be hard to have such a prominent name on github and npm and to have to deal with all those poor souls who have to use "old" bundlers ;-)

but thats off-topic....

i had the same issue with storybook and decided to remove the library, but i am curious if anyone knows the actual fix

@cristianoccazinsp
Copy link

@macrozone any drop-in replacement for it?

@Dean177
Copy link

Dean177 commented Feb 23, 2022

@cristianoccazinsp scroll up a little to my reply 😉

onlyling added a commit to 24jieqi/react-native-xiaoshu that referenced this issue Apr 7, 2022
@SakuOtonashi
Copy link

SakuOtonashi commented May 24, 2022

replace uglify-es with terser
install metro-minify-terser and edit metro.config.js.

// metro.config.js
module.exports = {
  transformer: {
    minifierPath: 'metro-minify-terser',
  },
}

@chriswiggins
Copy link

I stumbled into this issue today too, doing the following sorted the issue for me (YMMV):

npm install --save-dev @babel/plugin-proposal-numeric-separator

In the root of your project, create the file babel.config.js and add the following. This just extends metro's preset, and includes the numeric-separator transform

/**
 * Babel configuration override for React Native
 * Includes the @babel/plugin-proposal-numeric-separator plugin to fix _ numerical formatting
 *
 */

module.exports = {
  plugins: ['@babel/plugin-proposal-numeric-separator'],
  presets: ["module:metro-react-native-babel-preset"]
}

@Jorundur
Copy link

@chriswiggins Thanks, this fixed it for me! Simple solution.

santhoshvai added a commit to GetStream/stream-video-js that referenced this issue Nov 9, 2022
santhoshvai added a commit to GetStream/stream-video-js that referenced this issue Nov 9, 2022
santhoshvai added a commit to GetStream/stream-video-js that referenced this issue Nov 9, 2022
vanGalilea added a commit to GetStream/stream-video-js that referenced this issue Nov 11, 2022
* feat: make sfu call object to accept rn webrtc

* patch rtcrtpsender and rtcrtpreceiver and add polyfill for uuid

* chore: fix issues after merge with main

* fix: remove numeric symbols as its not compatible with metro for react-native

ref: facebook/metro#645

* feat: add healper to get username fragment in rn webrtc

* feat: add callID randomiser to RN

* feat: add callID randomiser to RN

* fix: typing issues with stream video client

* chore(react-native): remove unused modules

* fix: remove numeric symbols as its not compatible with metro for react-native

ref: facebook/metro#645

* chore(react-native): remove unused modules

* fix: remove numeric symbols as its not compatible with metro for react-native

ref: facebook/metro#645

* chore: remove usage of webrtc/types

* chore: remove the previously made generics for connection config and use global polyfilling

* feat: add polyfill of rn-webrtc

* feat: wip use participant videos from the shared state store

* feat: add timeout in measureResourceLoadLatencyTo

* chore: remove the temporary method that was used by react native

* chore: remove unnecessary console log

* fix: floating point numbers are not allowed in update subscriptions

* chore: remove unnecessary console log

* feat: filter out current user participant

* fix:  remove linking event listener

* chore: remove unused imports

* fix: patch crash in rn webrtc for undefined method on track

* chore: revert to using the staging urls by default

* chore: update yarn lock for webrtc patch

* chore: update the rn webrtc patch

* chore: remove the generics implementation in react-sdk

* chore: remove the generics implementation in react dogfood

* fix: latency call must get the blob of response

* chore: revert the changes done to sfu call object

* feat: patch ice candidate getter for react-native

* fix: if only local participant is present then show it as full view

* chore: align coordinator ws url

* chore: use same length to get random call id as react dogfood app

* fix: adding angular-sdk dir to metro.config blockList

Co-authored-by: vanGalilea <galiliziv@gmail.com>
@robhogan
Copy link
Contributor

robhogan commented Dec 20, 2022

This should be fixed by the release of Metro 0.73, which is bundled with React Native 0.71.

The root cause here is Metro's previous use of the uglify-es minifier, which can't parse numeric separators. I think numeric separators are supported by all of RN's runtime targets, we just needed a minifier that didn't choke on them.

In Metro 0.67 we introduced support for async minifiers, and therefore Terser 5 (#606). In Metro 0.73 we made Terser the default minifier (#871).

Using Terser with React Native 0.69.x and 0.70.x (Metro >= 0.67.0)

If you have a Metro version >= 0.67.0 but can't yet upgrade to RN 0.71 (still in RC), you can switch to Terser by modifying your minifierPath in Metro config:

Add metro-minify-terser to your project dependencies:

yarn add metro-minify-terser

Point Metro at it with minifierPath:

// metro.config.js
module.exports = {
  // ...
  transformer: {
    // ...
    minifierPath: require.resolve('metro-minify-terser')
  }
}  

For users on RN 0.68 or older, you can try the same config with yarn add metro-minify-terser@~0.66.2 (untested) - this will use Terser v4, which is synchronous and therefore should work with old Metro versions.

Closing, but feel free to reopen if anyone is still running into issues on RN 0.71 or later.

facebook-github-bot pushed a commit that referenced this issue Jan 11, 2023
…hen not in Hermes (#681)

Summary:
Changelog:
* **[Feature]**: Enable ES2021 numeric separator syntax in React Native

**Summary**

This PR solves issue #645. It adds `babel/plugin-proposal-numeric-separator` to the list of babel plugins when not using Hermes to more closely match ES2021. Now that ES2021 has been finalized, people will probably (?—I know I did) expect `1_000_000` to be treated as a valid number.

As I noted in a [comment](#645 (comment)) in #645, should `babel/preset-env` be included instead of including many different plugins manually? Even for engines like Hermes which don't need all the same transforms as JSC, I don't think the unneeded transforms would drastically increase the bundle size, right?

**Test plan**
Tested it using a sample app on `"react-native": "0.65.0-rc.2"`. Solves the `react-native bundle` issue on iOS (only tested on device as I encountered issues compiling for the simulator) and Android production / release builds when not using Hermes.

Pull Request resolved: #681

Reviewed By: rubennorte

Differential Revision: D29556107

Pulled By: motiz88

fbshipit-source-id: 2d66bf8993cb10dd1f020939f4ad1bbf49cccc19
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

No branches or pull requests