import-js / eslint-plugin-import

ESLint plugin with rules that help validate proper imports.
MIT License
5.55k stars 1.57k forks source link

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

Open rotu opened 5 months ago

rotu commented 5 months ago

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 commented 5 months ago

Notably they can't be string literals in JS before ES2021. and https://github.com/microsoft/TypeScript/issues/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 commented 5 months ago

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 commented 5 months ago

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)