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

False positive error for "Cannot use 'import.meta' outside a module" on a module file. #3639

Closed
mayfield opened this issue Mar 24, 2021 · 6 comments · Fixed by #3641
Closed

Comments

@mayfield
Copy link

Repro:

Include a module file that is dynamically inserted into the DOM.

Use an es6 module feature such as import.meta.url.

image
^^^ Image from validation report of extension being uploaded to AMO

This file is an es6 module and has a .mjs extension. It is loaded via JS with a type='module'.

The code is correct and works properly on all browsers including FireFox (of course).

Is there a way to hint the linter to avoid this rejection? I'm not able to release my new extension version because of this.

@mayfield
Copy link
Author

It appears that this only happens if the module has ONLY dynamic imports. If I include a dummy static import it triggers the linter into realizing it is a module. So the issue appears (on the surface) to be a ES6 module detection issue.

@Rob--W
Copy link
Member

Rob--W commented Mar 25, 2021

Please share a (minimal) test case to reproduce the issue. If i doubt, you can use the command-line version (or indirectly via web-ext lint) to check locally.

@rpl
Copy link
Member

rpl commented Mar 25, 2021

We should be already able to put together the minimal test case with the details provided in the first issue description and the comment that follows it.

The javascript scanner is currently going to lint a js file as module if it does "parse successfully as type module AND contain at least one import or export statement", see:

  • detectSourceType(filename) {
    // Default options taken from eslint/lib/linter:parse
    const parserOptions = {
    filePath: filename,
    sourceType: 'module',
    ecmaVersion: ECMA_VERSION,
    };
    const detected = {
    sourceType: 'module',
    parsingError: null,
    };
    try {
    const ast = espree.parse(this.code, parserOptions);
    detected.sourceType = this._getSourceType(ast);
  • _getSourceType(node) {
    const possibleImportExportTypes = [
    'ExportAllDeclaration',
    'ExportDefaultDeclaration',
    'ExportNamedDeclaration',
    'ExportSpecifier',
    'ImportDeclaration',
    'ImportDefaultSpecifier',
    'ImportNamespaceSpecifier',
    'ImportSpecifier',
    ];
    if (possibleImportExportTypes.includes(node.type)) {
    return 'module';
    }
    const keys = vk.KEYS[node.type];
    if (keys.length >= 1) {
    for (let i = 0; i < keys.length; ++i) {
    const child = node[keys[i]];
    if (Array.isArray(child)) {
    for (let j = 0; j < child.length; ++j) {
    if (this._getSourceType(child[j]) === 'module') {
    return 'module';
    }
    }
    } else if (child) {
    return this._getSourceType(child);
    }
    }
    }
    return 'script';
    }

I think we may either choose to:

  • prefer "module" if the the js file parse successfully as type module AND it's file extension is mjs
  • or prefer "module" if the js file parse successfully as type module (indipendently from its file extension)

The first option (preferring "module" for .mjs files) is probably the one with less potential side-effect and it may be reasonable enough.

@willdurand wdyt?

@willdurand
Copy link
Member

The first option (preferring "module" for .mjs files) is probably the one with less potential side-effect and it may be reasonable enough.

I agree.

@lucaseverett
Copy link
Contributor

lucaseverett commented Apr 3, 2022

Are there any workarounds besides renaming a file to .mjs? Maybe some dummy code or a comment I could add at the top of the file for it to be scanned as a module?

When I encountered this yesterday, I went the route of renaming some files to .mjs, but I made a mistake and it resulted in my addon's options page not working after release.

I use a build tool that doesn't allow me to specify file extensions, so I will always have to manually rename files after build to get around this bug, and manually renaming means the possibility of mistakes.

The Chrome release of my extension is built using the same code/repo, but the Chrome scanner didn't have any issue with the files being named .js. It's a little troubling to have to do this extra manual step for only Firefox.

@willdurand
Copy link
Member

willdurand commented Jul 25, 2022

Fixed by: #4399

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

Successfully merging a pull request may close this issue.

5 participants