Skip to content

Conversation

@ghost
Copy link

@ghost ghost commented Jul 17, 2018

Sequel to #23576.
I don't think it makes sense to view --noEmit as permission to suggest ES6 modules, because any JS project is likely to have --noEmit set.

@mhegazy
Copy link
Contributor

mhegazy commented Jul 17, 2018

@mjbvz have we heard any complains about the suggestion/quick fix to convert to ES6 modules being offered too often?

@mjbvz
Copy link
Contributor

mjbvz commented Jul 17, 2018

Yes previously there were quite a few node developers who did not like seeing the suggestion since it would end up breaking their code (if they were not using a transpiler)

@mhegazy
Copy link
Contributor

mhegazy commented Jul 17, 2018

Yes previously there were quite a few node developers who did not like seeing the suggestion since it would end up breaking their code (if they were not using a transpiler)

When you say "previously", you mean it is not an issue any longer? or are you still getting feedback about this?

For context #23576 disabled this suggestion for many projects. this change limits it further. wondering if we need to take this change, or we are in a good place with #23576 only.

@mjbvz
Copy link
Contributor

mjbvz commented Jul 17, 2018

No, when the suggestion was first introduced I heard a lot about it but I have not heard anything after we restricted when it will be shown with #23576

@RyanCavanaugh
Copy link
Member

Sounds like we're in a good place as-is. Easy enough to revive this if we hear more.

@ghost ghost deleted the noEmit_es6Modules branch September 5, 2018 18:34
@jarrodldavis
Copy link

I've run into this issue and I don't understand why the --noEmit flag has any bearing on the CommonJS-to-ES6 module suggestion. VS Code checks JavaScript files using TypeScript by default (which is what I want, but without this suggestion) and of course --noEmit is true since they aren't TypeScript files. This means that the restriction of the ES6 Modules suggestion has no effect for node.js JavaScript projects – the suggestion will be shown unless you manually set noEmit to false in jsconfig.json. That in turn causes another error to appear (Cannot write file '<file>' because it would overwrite input file.), so I have to set the outDir to a nonsense directory even though I have no intention of using it.

In short, I believe this pull request should have been merged since #23576 hasn't helped with JavaScript-only node.js projects, in my experience.

jarrodldavis added a commit to jarrodldavis/Learn-Node that referenced this pull request Jan 20, 2019
When editing JavaScript files in VS Code, TypeScript will suggest
converting any CommonJS module to an ES6 module, even for node.js
backend code. Unfortunately, how TypeScript determines when to suppress
or show this suggestion is flawed, especially in a combination
frontend/backend project like this one where the frontend code can use
ES6 modules through transpilation.

This adds a `jsconfig.json` file to the starter files that will suppress
the spurious suggestions for the node.js code, but maintain the
suggestion in the frontend code.

See these GitHub issues and pull requests for more details on the issue:

- microsoft/vscode#47299
- microsoft/vscode#47458
- microsoft/TypeScript#23391
- microsoft/TypeScript#23576
- microsoft/TypeScript#25721

- microsoft/TypeScript#25721 (comment)
- microsoft/TypeScript#23391 (comment)
@vdh
Copy link

vdh commented Jul 8, 2020

@RyanCavanaugh Can this please be reviewed? There's no guarantee that noEmit means someone is compiling. There's plenty of useful type-checking options that are gated behind compilerOptions:

  • tsc can be used to type-check JS files without compiling, but only supports tsconfig.json. Obviously if there's no compiling involved it makes sense to set noEmit to avoid generating output.
  • noErrorTruncation is not allowed in a jsconfig.json file.
  • Node 12 has a lot of ES2019 features (but not import syntax without that problematic experimental mode), so setting "target": "ES2019" now has this erroneous suggestion appearing.
  • Why shouldn't a Node environment be allowed to set "module": "CommonJS" to indicate the module type for type-checking purposes?

The logic around this suggestion seems to be quite negatively-geared against development scenarios where we need to stick with CommonJS due to Node scripting. I absolutely prefer to use import syntax when I'm allowed a compiler to support it, but it's absolutely maddening to be prompted on every file about this when working with Node scripts where I literally can not use the import syntax…

@microsoft microsoft locked as resolved and limited conversation to collaborators Oct 21, 2025
@typescript-bot
Copy link
Collaborator

This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants