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

jsconfig default settings for module type is inconsistent #6690

Closed
egamma opened this issue Jan 28, 2016 · 10 comments
Closed

jsconfig default settings for module type is inconsistent #6690

egamma opened this issue Jan 28, 2016 · 10 comments
Labels
Duplicate An existing issue was already created Suggestion An idea for TypeScript

Comments

@egamma
Copy link
Member

egamma commented Jan 28, 2016

No jsconfig.json in the workspace
Then the module type defaults to none

jsconfig.json exists but no module property defined
Then the module type defaults to commonjs

Suggest that the module type default is always the same independent of whether there is a jsconfig.json.

If the module property is commonjs by default then I suggest that there is also a default for exclude containing the node_modules folder.

@egamma egamma added the Salsa label Jan 28, 2016
@zhengbli zhengbli added the Bug A bug in TypeScript label Jan 28, 2016
@zhengbli zhengbli self-assigned this Jan 28, 2016
@mhegazy
Copy link
Contributor

mhegazy commented Jan 28, 2016

the problem here was you will get an error:

error TS1148: Cannot compile modules unless the '--module' flag is provided. Consider setting the 'module' compiler option in a 'tsconfig.json' file.

which most ppl have found annoying int he past. given that JS users would probably have a transpiler already, do we want to show them that error?

@zhengbli
Copy link
Contributor

@mhegazy Does't the default module option also impact how we resolve modules for js files? Even without the error message

@egamma
Copy link
Member Author

egamma commented Jan 28, 2016

@mhegazy the error message is unpleasant since I don't think of code generation when I use JS only. However, given your argument, shouldn't then the default also be commonjs when there is no jsconfig.json?

@mhegazy
Copy link
Contributor

mhegazy commented Jan 28, 2016

that is correct, but for that we could set the "moduleResolution" flag, but the error still stays.

@mhegazy
Copy link
Contributor

mhegazy commented Jan 28, 2016

shouldn't then the default also be commonjs when there is no jsconfig.json?

in retrospect, yes :) but this would be a big breaking change.

@billti
Copy link
Member

billti commented Jan 28, 2016

I sent an email on this couple mins ago... repeating below

We chose the current behavior as without a ‘module’ setting the TypeScript engine gives you errors on import/export statements that no module type is defined. With an ES3/ES5 emit target (the default and most common settings), we need to know how to treat these (e.g. as CommonJS, AMD, or System modules). With JavaScript also (optionally) being transpiled now, the same problem exists.

We initially did have a default other than ‘none’, but found too many users were trying to use modules without understand them and the need for a runtime that supported them, so we wanted to add a little impedance to make them stop and think “what module type are trying to use”, and maybe look at some docs if not sure.

Some of the user confusion was caused by TypeScript overloading the “module” keyword for both internal & external modules. Now we’ve moved to “namespace” for internal modules, this should be lessened. Also, Node development being more prevelant, and with CommonJS also being commonly the format of choice in the browser (via browserify, WebPack, etc.), perhaps we could consider defaulting to CommonJS again. Thus ES6 import/export syntax just works, and would emit CommonJS by default, if there is no “module” setting provided.

@mhegazy
Copy link
Contributor

mhegazy commented Jan 28, 2016

in retrospect, yes :) but this would be a big breaking change.

correction for my original statement, that would not be a breaking change, existing projects should not be affected. it would be a surprising change though to existing users of TS.

We should consider this for the design meeting tomorrow.

@zhengbli zhengbli added In Discussion Not yet reached consensus and removed Bug A bug in TypeScript labels Jan 28, 2016
@mhegazy mhegazy added Suggestion An idea for TypeScript Committed The team has roadmapped this issue labels Jan 29, 2016
@mhegazy
Copy link
Contributor

mhegazy commented Jan 29, 2016

Conclusion is to move to CommonJs as the default module kind

@mhegazy mhegazy removed the In Discussion Not yet reached consensus label Jan 29, 2016
@mhegazy mhegazy added this to the TypeScript 2.0 milestone Jan 29, 2016
@egamma
Copy link
Member Author

egamma commented Feb 1, 2016

a good outcome, thanks.

Should this also be assigned to the Salsa 0.9 milestone?

@mhegazy mhegazy modified the milestones: Salsa 0.9, TypeScript 2.0 Feb 1, 2016
@mhegazy
Copy link
Contributor

mhegazy commented Feb 2, 2016

closing in favor of #6807

@mhegazy mhegazy closed this as completed Feb 2, 2016
@mhegazy mhegazy added Duplicate An existing issue was already created and removed Committed The team has roadmapped this issue labels Feb 2, 2016
@mhegazy mhegazy modified the milestones: Salsa 0.9, TypeScript 1.8.2 Feb 2, 2016
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate An existing issue was already created Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

4 participants