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

error index.js: "" is not a valid identifer name error with 7.7.0 #10645

Closed
francois-codes opened this issue Nov 5, 2019 · 19 comments · Fixed by #10650
Closed

error index.js: "" is not a valid identifer name error with 7.7.0 #10645

francois-codes opened this issue Nov 5, 2019 · 19 comments · Fixed by #10650
Labels
i: bug i: regression outdated A closed issue/PR that is archived due to age. Recommended to make a new issue

Comments

@francois-codes
Copy link

Bug Report

Hi There,
One of our builds on CI has just failed when we picked up the latest 7.7.0 release.
Building a react-native project is failing and we're getting this error :

error index.js: "" is not a valid identifer name
debug TypeError: "" is not a valid identifer name
    at node_modules/metro/node_modules/@babel/types/lib/definitions/core.js:322:17
    at Object.validate (node_modules/@babel/types/lib/definitions/utils.js:201:7)
    at validateField (node_modules/metro/node_modules/@babel/types/lib/validators/validate.js:22:9)
    at validate (node_modules/metro/node_modules/@babel/types/lib/validators/validate.js:16:3)
    at builder (node_modules/metro/node_modules/@babel/types/lib/builders/builder.js:38:27)
    at Object.Identifier (node_modules/metro/node_modules/@babel/types/lib/builders/generated/index.js:334:31)
    at functionFromProgram (metro/src/ModuleGraph/worker/JsFileWrapping.js:71:7)
    at Object.wrapModule (metro/src/ModuleGraph/worker/JsFileWrapping.js:39:19)
    at node_modules/metro/src/JSTransformer/worker.js:323:52

This happend on circle CI right after 7.7.0 was published, on a code base that hasn't changed, and is building perfectly on other platforms where the dependencies haven't been installed.

The problem is that many packages (like the metro bundler) have @babel/core@^7.0.0 defined as dependency, so this is automatically picked up when using these dependencies. This is what happened to us this morning

Current Behavior
I expect my RN bundle to build 😬

Input Code
simply make sure 7.7.0 is resolved. if 7.6.3 or earlier is resolved, all is good

Environment

  • Babel version(s): [7.7.0]
  • Node/npm version: [Node 8]
  • OS: [Ubuntu]
  • Monorepo: [no]
  • How you are using Babel: [react native metro bundler]

Possible Solution
remove that update

@babel-bot
Copy link
Collaborator

Hey @f-roland! We really appreciate you taking the time to report an issue. The collaborators on this project attempt to help as many people as possible, but we're a limited number of volunteers, so it's possible this won't be addressed swiftly.

If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack community that typically always has someone willing to help. You can sign-up here for an invite."

@francois-codes
Copy link
Author

this is fairly urgent, as I believe 7.7.0 has unvoluntarily introduced a breaking change
this is where it happens :

throw new TypeError(`"${val}" is not a valid identifer name`);

somehow this must not be called properly, as when I logged the params after an error, here's what i have

node = { type: 'Identifier', name: '' }
key: "name"
val: ""

@guyca
Copy link

guyca commented Nov 5, 2019

For now, downgrading to "@babel/types": "7.6.x" seems to resolve it

@francois-codes
Copy link
Author

@guyca not when the dependency is pulled from another package which defines @babel/core: ^7.7.0

@francois-codes
Copy link
Author

@guyca besides, if you have to rollback for a feature to work, it means there's a breaking change in the version, and it shouldn't be 7.7.0 but 8.0.0 and we wouldn't have that problem
@nicolo-ribaudo this is really serious. Could you please - at least consider to - unpublish that version so it doesn't block people from building react-native bundles ?

@nicolo-ribaudo
Copy link
Member

nicolo-ribaudo commented Nov 5, 2019

We will publish a fix today (In about 2 hours).

PS. I suggest using a lockfile to avoid problems with regressions like this one.

@francois-codes
Copy link
Author

We have lockfiles. But in some environments, you can generate a project and run yarn start on it, and therefore no lockfile.

This is a semver violation !

@francois-codes
Copy link
Author

Thanks @nicolo-ribaudo. When will the faulty versions be unpublished from npm ?

@yuanfux
Copy link

yuanfux commented Nov 5, 2019

FYI, react native community is deeply affected by this.

@francois-codes
Copy link
Author

Thanks @yuanfux - still waiting for the versions to be removed from the npm registry - that's the only real fix

@francois-codes
Copy link
Author

@nicolo-ribaudo this isn't resolved as long as the version is still on npm. Who can unpublish it ?

@nicolo-ribaudo nicolo-ribaudo reopened this Nov 5, 2019
@nicolo-ribaudo
Copy link
Member

https://www.npmjs.com/package/@babel/types/v/7.7.1

I will update the changelog later.

@francois-codes
Copy link
Author

@nicolo-ribaudo I don't think that will do... now the projects which have captured that in their cache won't work either... the best way to address this is to unpublish the version, not create a new one

@francois-codes
Copy link
Author

@nicolo-ribaudo confirmed that 7.7.1 works. Still think it'd be better to remove the faulty version though. NPM gives you 3 days after the initial publish for that. After that, it's too late to remove !

Anyway, thank you for resolving this fairly quickly. But it had a very big impact on a big chunk of the JS community, especially react-native developers

@loganfsmyth
Copy link
Member

loganfsmyth commented Nov 5, 2019

Definitely an unfortunate Metro bug: https://github.com/facebook/metro/blob/690f175c2d88432521edd9cde55bfdd220588134/packages/metro/src/ModuleGraph/worker/JsFileWrapping.js#L81

An empty string is not a valid variable name and isn't a valid function name :( They should just be passing null, e.g.

  return t.functionExpression(
    null,
    parameters.map(makeIdentifier),
    t.blockStatement(program.body, program.directives),
  );

Tracked in facebook/metro#481

@dekameron22
Copy link

and what with @babel/core?
I'm using
"@babel/core": "^7.7.0",
"@babel/runtime": "^7.7.0",

@mxmzb
Copy link

mxmzb commented Nov 5, 2019

in my case the error has not gone away yet even with @babel/types@7.7.1. will come back here when i find the issue.

// update: found the issue. after removing node_modules and yarn.lock it now works.

@francois-codes
Copy link
Author

@loganfsmyth fair enough, but still, the bundle build failures happened with babel 7.7.0
fixing that bug on metro and keeping 7.7.0 as it was means you're effectively deprecating all versions of RN but the last that will use the latest metro update

@loganfsmyth
Copy link
Member

@f-roland Yep! My intention was only to provide context, not to argue against reverting this change.

@lock lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Feb 4, 2020
@lock lock bot locked as resolved and limited conversation to collaborators Feb 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
i: bug i: regression outdated A closed issue/PR that is archived due to age. Recommended to make a new issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants