| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
Can we opt-out of this change in emit via a compiler flag? |
Sorry, something went wrong.
|
Andrew Bradley (@cspotcode): Opting out of the emit change should be as simple as using noEmitHelpers and providing noop functions for the new helpers (minimally, we need to discuss if we want to offer more). The change to make * as Foo uncallable from the type system's perspective... It's a spec bug we've had for awhile and we've tried to discourage it. We'll probably offer a quick fix, but an opt-out flag we need to discuss. |
Sorry, something went wrong.
|
Daniel Rosenwasser (@DanielRosenwasser) As you suggested, I've added the same changes to the AMD emit (babel does also do this for their AMD emit, so it certainly makes us more consistent). |
Sorry, something went wrong.
|
After a bit of discussion, we've decided that even with the quickfix available, we'd like this to be behind a flag for a release, then made into the default but still opt-out, then finally made the only option in a 3rd release (also known as a proper deprecation cycle). As such, I've gone ahead and updated the checker behavior - allowSyntheticDefaultImports is now the default behavior (though you can still force it off if you're overwriting the emit helpers), and it now has much safer behavior which aligns with the emit more often (at the cost of greater complexity). For example:
Right now everything in this PR is still on by default, however I should have it behind a flag in short order - without that it's still likely ready for initial review and iteration. |
Sorry, something went wrong.
…ood error recovery with a quickfix
|
Thanks for working on this. Excited for babel & typescript to behave in similar fashion when importing non es6 modules. |
Sorry, something went wrong.
| category: Diagnostics.Module_Resolution_Options, | ||
| description: Diagnostics.Allow_default_imports_from_modules_with_no_default_export_This_does_not_affect_code_emit_just_typechecking | ||
| }, | ||
| { |
There was a problem hiding this comment.
This is not a strict option per se.. this is an interop flag.. i would not use strict prefix since we have already given it a different meaning with --strict.
Sorry, something went wrong.
There was a problem hiding this comment.
🚲 🏠 🕐 Alright, --standardESM then?
Sorry, something went wrong.
There was a problem hiding this comment.
Spoke with daniel, gunna try --ESMInterop
Sorry, something went wrong.
|
Mohamed Hegazy (@mhegazy) Changed the option to the name we discussed at today's design meeting, and set it to output as true in the --init default object, also as discussed. |
Sorry, something went wrong.
|
Hi, I created a demo repository for ESM using your module-nodejs branch. When using weswigham@7ff11bb I get the correct output for cjs: "use strict";
var __importDefault = (this && this.__importDefault) || function (mod) {
return (mod && mod.__esModule) ? mod : { "default": mod };
}
exports.__esModule = true;
var hybrid_1 = require("hybrid");
var path_1 = __importDefault(require("path"));
function run() {
console.log("ESM is getting real");
console.log(path_1["default"].posix.join("i", "can", "use", "cjs", "modules"));
console.log("And hybrid modules:");
console.log(hybrid_1.sayHello());
}
run();But when I use weswigham@bcafdba with ESModuleInterop I get the following (invalid) code when building cjs: "use strict";
exports.__esModule = true;
var hybrid_1 = require("hybrid");
var path_1 = require("path");
function run() {
console.log("ESM is getting real");
console.log(path_1["default"].posix.join("i", "can", "use", "cjs", "modules"));
console.log("And hybrid modules:");
console.log(hybrid_1.sayHello());
}
run();
It seems that the rename caused the option to no longer be applied. |
Sorry, something went wrong.
|
Wesley Wigham (@weswigham) can you also update https://github.com/Microsoft/TypeScript-Handbook/blob/master/pages/Compiler%20Options.md |
Sorry, something went wrong.
Sorry, something went wrong.
|
Having read the description, it's not fully clear to me whether this is a breaking change or not. If it is, it probably has a very large impact. Could someone clarify? |
Sorry, something went wrong.
|
This does not affect TS without the esModuleInterop flag specified beyond small enhancements to check if a JS file has an explicit __esModule marker while under allowSyntheticDefailtImports. Without setting esModuleInterop you shouldn't notice any changes at present. But we would like to make this behavior the default in the future (to align our default emit with babel), so you should check if this flag breaks you. If it does, you have a future problem that needs fixing - likely invalid usage of the namespace import hack that you should probably use a default import for instead (which was not possible prior to the addition of the emit helpers this flag enables). There is a quick fix to that effect bundled with this PR, so finding and fixing all those instances shouldn't be hard. 🌞 |
Sorry, something went wrong.
|
how to work with when "esModuleInterop": true,? globby is a export = function like module import * as globby from 'globby'; var __importStar = (this && this.__importStar) || function (mod) {
if (mod && mod.__esModule) return mod;
var result = {};
if (mod != null) for (var k in mod) if (Object.hasOwnProperty.call(mod, k)) result[k] = mod[k];
result["default"] = mod;
return result;
}
Object.defineProperty(exports, "__esModule", { value: true });
const globby = __importStar(require("globby"));
TypeError: globby is not a function |
Sorry, something went wrong.
|
Use a default import for export= modules that aren't namespace like: import globby from 'globby' |
Sorry, something went wrong.
|
no option disable importStar only? |
Sorry, something went wrong.
|
I don't think so, no. But a default import aught to work for you in this case; since it's not a module it's top level exported object becomes it's default. You shouldn't need to wrap the library or anything; simply using the other import syntax should suffice. |
Sorry, something went wrong.
|
thx, feel sad, looks like esModuleInterop is not good for me i hope can import * is old way ( make sure everything is expected ) lookes like i should keep use old option lol |
Sorry, something went wrong.
|
bluelovers (@bluelovers) as far as I can tell, you shouldn't see bad runtime behavior without getting a build time error anyway, and import star is less correct: import default is not really correct either, but import star is not used to get a commonjs export object outside of the typescript world. Wesley Wigham (@weswigham) TS has a logically correct import name = require("id") form, but it doesn't allow using this when targeting ES2015 modules. Any chance this restriction could get lifted? Targeting esmodules for your own code for bundling/tree-shaking purposes while still using commonjs modules makes sense. |
Sorry, something went wrong.
|
anthor question why we need __importStar not just require when esModuleInterop open? just confused |
Sorry, something went wrong.
In both node's current esm environment and in the browser's esm environment, there's no concept for an import name = require("id") statement to transpile to. Without a real definition for what the target environment is, saying that we can transpile it is a dicey prospect (esp. since how you expect it to "behave" in your case is entire based on how your toolchain is composed). You could always transform away imports of that style yourself in a before transform as part of your toolchain and ignore the error saying it's forbidden.
If you didn't need it, than you wouldn't have to use the esModuleInterop flag. All it does is alter the emit and typechecking to make commonjs packages available as a "default" import, which is how babel does commonjs interop. |
Sorry, something went wrong.
|
Wesley Wigham (@weswigham) Pretty sure node esm can require() commonjs modules? It's just a function, after all. They call out the other way around, not being able to require a mjs file, which makes some sense. And you do have a definition for your environment: if @types/node or @types/react-native or whatever isn't referenced, you already get an error about require not being defined, so you can look at the modules option the same as Babel's module transforms, and this import just being a version of const that pulls in the module typings. The use would mostly be clarity about behavior, but with this it also allows skipping the interop cost. |
Sorry, something went wrong.
|
It can't. require isn't available in node's esm mode. |
Sorry, something went wrong.
|
Not that node's esm mode is stable in the least, so that's still got the potential to change. |
Sorry, something went wrong.
|
oh I thought esModuleInterop is for import default there has something we need use ? import * as a form 'a'; //.... ? a.default |
Sorry, something went wrong.
|
That same default import needs to be available on the namespace and named imports, too. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Fixes #16093
This PR updates our commonJS and AMD (thus by extension UMD) module emit to synthesize namespace records based on the presence of an __esModule indicator, or lack thereof, bringing our emit in line with babel's. (Modules which are not babel-compiled will finally be uncallable and unconstructable unless you use the synthetic-default-import version of the module.)
This changes:
Will break. The correct way to write this code (with allowSyntheticDefaultImports to handle the type changes) is:
This also: