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

arbitrary-module-namespace-identifier is showing an error in tests #3010

Open
rotu opened this issue May 22, 2024 · 3 comments
Open

arbitrary-module-namespace-identifier is showing an error in tests #3010

rotu opened this issue May 22, 2024 · 3 comments

Comments

@rotu
Copy link

rotu commented May 22, 2024

When running the tests locally, I see an odd error:

$ npm run test
...
Error while parsing D:\Source\eslint-plugin-import\tests\files\no-unused-modules\arbitrary-module-namespace-identifier-name-a.js
Line 2, column 17: Line 2: Unexpected token

  1 | const foo = 333
> 2 | export { foo as "foo" }
    |                 ^
  3 |
`parseForESLint` from parser `D:\Source\eslint-plugin-import\node_modules\babel-eslint\lib\index.js` is invalid and will just be ignored

Module export names can indeed be string literals in JavaScript.

But note that module export names cannot be string literals in TypeScript, and it seems the TypeScript compiler also mangles these.

@ljharb
Copy link
Member

ljharb commented May 22, 2024

Notably they can't be string literals in JS before ES2021. and microsoft/TypeScript#40594 is the TS issue to support them.

Since I haven't dug into this further yet, I'll say that if the test case shouldn't be running (because the failure is expected) then it should be skipped when expected to fail; if it should be working, then it should be fixed :-)

@rotu
Copy link
Author

rotu commented May 22, 2024

Thanks! I was about to file an issue with TypeScript - don't know why I couldn't find it :-).

I don't know what the expectation is here - the babel-eslint package is deprecated in favor of @babel/eslint-parser "babel-eslint is now @babel/eslint-parser. This package will no longer receive updates." (I don't know whether newer versions parse this correctly either); is there a way to indicate when running tests which of these dependencies to use?

@ljharb
Copy link
Member

ljharb commented May 22, 2024

The solution I'd like there is attempted in PRs like #2482 - basically, run each test case in multiple parsers, transparently, and the test case only defines the language features it's using. However, I don't think anything's using this technique here yet (eslint-plugin-react uses it extensively already)

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

No branches or pull requests

2 participants