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

Not working for .mjs files #2

Closed
jpike88 opened this Issue Feb 1, 2018 · 10 comments

Comments

Projects
None yet
2 participants
@jpike88
Copy link

jpike88 commented Feb 1, 2018

Node using experimental-models flag requires es6 files are named .mjs

Deepscan isn't caring about them

@cimfalab

This comment has been minimized.

Copy link
Contributor

cimfalab commented Feb 1, 2018

Thank you for your issue and pull request for it!
I'll discuss it with our team and get back to you.

@cimfalab cimfalab self-assigned this Feb 2, 2018

@cimfalab cimfalab added the enhancement label Feb 2, 2018

@cimfalab

This comment has been minimized.

Copy link
Contributor

cimfalab commented Feb 2, 2018

Dear @jpike88 ,
We have just discussed for the support of .mjs file.

[TL;DR] We are not going to apply this change for now. 😓

Just changing the file extensions in the VS Code extension does not fully work.
It is also necessary to change the analyzer to support .mjs file because the VS Code extension sends the file to our server and actual analysis is done by the analyzer on the server.

But the analyzer is not going to include .mjs file for now by two reasons:

  • .mjs is experimental feature and might be official on Node v10 (LTS on August)
  • ESLint also is arguing for its application

So we would like to review the support of .mjs file after the release of Node v10.

I appreciate your understanding.
Thank you again for your contribution! 👍
(And thanks for this commit 4c95e31)

@jpike88

This comment has been minimized.

Copy link
Author

jpike88 commented Feb 2, 2018

That feature might be ‘experimental’ But I’ve been using it in a large scale production environment for over 6 months. I can assure you that it’s so useful that the node foundation won’t be modifying it much from what it already is... besides, it’s just ES6 JavaScript, nothing special beyond that. If you treat it as ES6 I guarantee you won’t have any issues.

@jpike88

This comment has been minimized.

Copy link
Author

jpike88 commented Feb 2, 2018

At least having it as a feature you activate with a workspace settings flag would be ideal. .mjs is just a way to say to Node ‘treat me differently at runtime initialisation’. It has no special syntax requirements.

@jpike88

This comment has been minimized.

Copy link
Author

jpike88 commented Feb 2, 2018

Also that argument JSLInt is having is about adding .mjs as a DEFAULT flag, not about whether or not support it (they already do)... would recommend you guys reconsider your decision.

@cimfalab

This comment has been minimized.

Copy link
Contributor

cimfalab commented Feb 2, 2018

Okay, thank you for your thoughtful comments.

I am going to provide an option like "Additional JavaScript File Suffixes" in the extension.
And when it's given, the extension will send the file with ".js" suffix like "test.mjs.js".
Then, our analyzer can handle that without a change.

When it's done, I will leave a comment.
Thank you!

@jpike88

This comment has been minimized.

Copy link
Author

jpike88 commented Feb 2, 2018

Perfect, I use experimental modules everywhere in my server code and they're amazing. Would recommend checking it out if you guys find some time.

Thanks

@cimfalab cimfalab closed this in 0d5ae36 Feb 5, 2018

@cimfalab

This comment has been minimized.

Copy link
Contributor

cimfalab commented Feb 5, 2018

Hello, I just published a 1.5.1 extension.
It provides a setting 'deepscan.fileSuffixes', so you can set like:

    "deepscan.fileSuffixes": [
        ".mjs"
    ],

After restarting VS Code, you can inspect .mjs file (The extension tries to send .mjs file as .js file to the server).
Feel sorry to say that you need restart because I could not find the way to dynamically change DocumentSelector for LanguageClient.

I hope that you've found this update helpful.

@jpike88

This comment has been minimized.

Copy link
Author

jpike88 commented Feb 5, 2018

Helpful? I freaking love you guys!

Thanks heaps.

@cimfalab

This comment has been minimized.

Copy link
Contributor

cimfalab commented Feb 22, 2019

DeepScan now supports ECMAScript Modules file (.mjs) by default.
So you don't need this "deepscan.fileSuffixes" option for .mjs files any more.
I recommend you:

  • Update to the latest extension (1.8.0 and later)
  • Remove the "deepscan.fileSuffixes" option (If the option is set for ".mjs", it will be analyzed as a regular JavaScript file instead of a module file.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.