Skip to content
This repository has been archived by the owner on Sep 2, 2023. It is now read-only.

Use Case #42 Discussion #80

Closed
weswigham opened this issue May 9, 2018 · 45 comments
Closed

Use Case #42 Discussion #80

weswigham opened this issue May 9, 2018 · 45 comments

Comments

@weswigham
Copy link
Contributor

At today's meeting, some people expressed interested in a more complete description of the context and background around use case 42. Here's the usecase, for reference:

Rachel is a TypeScript user who is importing some JavaScript code that uses CommonJS. She uses declaration files that were written on DefinitelyTyped, but were authored as ES module top-level exports as so:

export function foo() {
}
export function bar() {
}

When she imports them from TypeScript, she gets code-completion on the namespace import for foo and bar

import * as thing from "some-package";
thing./* completions here*/

When she compiles her code to run on either the 'commonjs' or 'esnext' module targets, she expects both to run correctly.

So, first, some background. TypeScript has these things called "declaration files". They're additional metadata about a .js file that includes additional type information for a module (written in files with a .d.ts extension); this is how vscode can provide good completions for things like lodash and jquery. They usually look something like this:

// Type definitions for abs 1.3
// Project: https://github.com/IonicaBizau/node-abs
// Definitions by: Aya Morisawa <https://github.com/AyaMorisawa>
// Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped
/**
 * Compute the absolute path of an input.
 * @param input The input path.
 */
declare function Abs(input: string): string;
export = Abs;

or this:

// ... some more definitions
export * from "./createBrowserHistory";
export * from "./createHashHistory";
export * from "./createMemoryHistory";
export { createLocation, locationsAreEqual } from "./LocationUtils";
export { parsePath, createPath } from "./PathUtils";
export { createBrowserHistory, createHashHistory, createMemoryHistory };

that is to say, normal es6 syntax with the addition of export= (to describe the commonjs only pattern of replacing the namespace object) and type annotations, without any expressions or blocks. Someone went and put in the effort to write these type definitions at some point in the past, and there's now a community of people authoring these and keeping them up-to-date. The hub of that community is DefinitelyTyped - every definition file published there is automatically published to the @types npm namespace under the same name as the package it corresponds with - this means that, for example, jquery has types available via @types/jquery. It's a kind of crowd-sourced documentation/metadata store.

So, the dilemma. The typescript compiler (and by extension the vscode js language service, as it is really just the typescript compiler behind a thin facade) follows node's module reolution scheme to find js and/or ts files. In addition, it will also look for an adjacent .d.ts file to provide type information for a .js file, and, failing that, an @types/packagename package with a declaration file to provide the types. (Failing either of those in some configurations it will fall back to the actual JS, if it is able to, but this is costly - there's a lot of JS and it needs to be processed a lot to get good type data from it, which is why declarations are preferred.) We have two unique issues to deal with in the esm transition, both of which come into play here in this use-case. The simpler one is emit - providing a node-esm emit target that interoperates decently. The more complicated one is typechecking.

To start with typechecking (for both js files and ts ones): You'll note in my description of declaration files above, I didn't mention anything about any encoding of the library's available module format(s). This is important - we expect that no matter if you're targeting cjs or amd or esnext that the same declaration file will be able to accurately represent the module. This is critical, as it turns out, because some of our consumers will target esnext with us, but then actually transpile to cjs using another bundling tool, like rollup or webpack (retaining the es6 style imports for dead code elimination). We (strongly) operate under the assumption that interop between various module formats is invisible to the end-user - this carries into the js editing experience, where we assume that weather you wrote import * as x from "lodash" or const x = require("lodash") it will produce roughly the same module[1], and have the same members when you look for completions on x. Now, clearly we're capable of changing this assumption (likely behind a flag, but w/e), but this would (will?) fracture our ecosystem of existing type definition files; anything already written would need to only be interpreted as a cjs package, and we'd have to introduce a marker (file extension, pragma, or otherwise) to flag a declaration file as node-esm so that we can reject the old one and only accept the other for resolution depending on the exact interop scheme. It's not exactly pretty, and goes about as far away from a single "universal" declaration file as you can get (and, naturally, starts to require extra maintenance work to maintain the doubled APIs). Compound that with the fact that nobody usually bothers to tell their editor anything about the files they're working with (ie, will this random .js file be targeting node-esm, esm, or cjs? - at least at first), and we might really have to start arbitrarily guessing about what types an import should actually map to on disk, depending on any exact interop scheme, which is no good from a type safety perspective.

Our emit issues are more clear, and mostly center around exactly how interop might work. The typescript compiler, being a transpiler with multiple supported output formats, allows you to write the same es6-style input code and transpile it to either cjs or esm module formats (or amd or umd or systemjs). It will also auto-generate a declaration file for you. Generally, it is expected that your code will function the same way when targeting any of these module runtimes and present the same interface to consumers who can understand the format (and the same declaration file is currently produced for all of them). Some constructs (like export namespace assignment) aren't supported on some emit targets (ie, esnext), but otherwise interop is generally expected (after all, that's a big part of a transpiler's job). Node's interop scheme, if not fully transparent, would probably require us to emit helpers/perform transforms mapping from the more transparent interop we support today to any more explicit form of module system interop supported by the platform, thus requiring a new, independent module target, different from normal un-transformed esnext modules. Failing that, it would require a flag that at least alters our checking and resolution to only allow any stricter platform interop scheme, which would, naturally, not be able to be the default so as to not break long time users.

We also have relatively strong compatibility needs, since our service needs to keep working on code that was written 1, 2, 3 years ago, as nobody wants to launch their editor on their legacy codebase and just be greeted with a bunch of confused warnings and incorrect types, which necessitates a lot of changes be non-default flags. Our stance is, typically, we only intentionally introduce "breaks" if said break is identifying real problems in your code (ie, you were accessing a property which could never actually exist, but we didn't catch before). And then we'll usually have a flag to turn off the new check. Even for the 3.0 release that we have a milestone up for now, we don't have any really major breaks in the backlog - just larger features (it's more of a pragmatic answer to "what comes after 2.9 when incremented by .1" than a semver major release).

[1]We have a flag for placing the module on an es6-style import's default and disallowing calling namespace objects (esModuleInterop), to align our typecheck and emit with babel's emit, however this isn't currently our default for fear of breaking longtime users.

cc @DanielRosenwasser I hope I've explained your concerns, but you should feel free to chime in.

@guybedford
Copy link
Contributor

guybedford commented May 12, 2018

Here are my suggestions on the above. I hope this makes some sense and please let me know if I'm missing anything at all here. I'm an avid user and huge fan of TypeScript and use it in all my work, and I I would love to see these interops working smoothly.

As you say a flag of sorts is likely necessary to change from the full freedoms in use today to something closer to the NodeJS interop.

Perhaps strictNodeInterop: true or similar, enabling a workflow where:

import { x } from 'cjs'; // fails
import x from 'cjs'; // provides the CommonJS
import { x } from 'es-module'; // works, because the resolver informs the type checker this is an ES module

The nice thing about such a mode is that it simplifies the emission a lot - the emission is guaranteed to work out. As you say, otherwise you need to do some fancy destructuring stuff to continue to make things work in the emission (import __cjs from 'cjs'; const { x } = __cjs;) which I would still expect to happen by default when not using the strictNodeInterop flag.

Now it may well still turn out that we magically get TC39 to allow module namespaces to define named exports post-execution. And if this happens, then the above is no longer a problem for TypeScript. Note though that semantically distinguishing CommonJS and ES modules as having different values for import * and require('x') is probably still important here in that import * for CommonJS must be a copy of the named exports, with a default added for good measure, while the require('x') value is the live module.exports binding.

Emission for legacy module formats pretty much remains the same here entirely in that the Babel interop applies (__esModule checking etc), and that will all continue to work out well since NodeJS commonJS modules likely won't be able to import ES modules to create any second-order double-interop problems.

The rules for the type checker should probably follow similarly:

  1. Ideally fix up the ability to support import x from 'cjs' alongside import { x } from 'cjs' by having the resolver know the format of the module loaded. This way users can have proper interop coexist side-by-side with the named exports sugar (which is right now my biggest complaint and pain with TypeScript actually). We split into two cases: CJS and ES modules, where the exports for CJS modules are taken to have the form interface CJSModule { default: CJSModuleDefinitelyTypedDefinition, ...CJSModuleDefinitelyTypedDefinition } where the named exports are extended over the namespace instead of treated as the namespace and default is never overridden. This I think would be preferable to the treatment of synthetic default imports today which seems to switch to one mode or the other instead of having both coexist.
  2. Enable a strictNodeInterop mode on the above that then just cuts out the named exports sugar. Or maybe we are in luck with the spec, and don't even need to do that!

I know there's a lot more nuance here to the typechecker, and I'm more than glad to delve into the details here further. Please also let me know if I've missed any concerns here too. And thanks for the great work you guys do.

@guybedford
Copy link
Contributor

Also similarly here, now that nodejs/node#20403 has landed, it is possible in NodeJS to support:

x.mjs:

import fs, { readFile } from 'fs';

So ideally TypeScript should be able to support this as well now - again this would be enabled by the same thing described in (1) above - supporting CommonJS modules as having their iterable properties extended over a module namespace object, with the default as the definition itself.

@weswigham
Copy link
Contributor Author

@guybedford The contention isn't that it wasn't possible to wire up a mode that works with node-esm (it's always possible with enough flagging and elbow grease), it's that it'd be a break and would require behavioral changes we couldn't make as defaults, and on top of that would require invalidating the entire existing ecosystem in some interop cases, which makes the editing experience very bad. Though in these same cases where we'd need to flag things and invalidate old declarations as esm, the node ecosystem also fractures (as esm won't be substitutable for cjs to retain API compatibility), so it's almost expected (just not good for... almost anyone) 🤷‍♀️

@weswigham
Copy link
Contributor Author

weswigham commented May 12, 2018

Also similarly here, now that nodejs/node#20403 has landed

And that actually complicates things even more, since its' cjs but doesn't act like other cjs (since normal cjs can't produce named exports rn), so is effectively a 3rd kind of module that needs to be worked into resolution and declarations somehow.

@guybedford
Copy link
Contributor

@weswigham my suggestion is exactly to enable by default an interop that is both backwards and forwards-compatible supporting default and named exports for CommonJS side-by-side. This provides a non-breaking upgrade path, where the strictNodeInterop flag then only acts as the final "deprecation" step to align with Node interop.

It's really important to enable the support in TypeScript today for users to write forward-compatible code by default (using the default :P), as stopping that will continue hold the entire ecosystem back.

@devsnek
Copy link
Member

devsnek commented May 12, 2018

it seems like the summary of this is that people want an api to resolve and get the named exports of a file by node's loader's rules?

@weswigham
Copy link
Contributor Author

@weswigham my suggestion is exactly to enable by default an interop that is both backwards and forwards-compatible supporting default and named exports for CommonJS side-by-side. This provides a non-breaking upgrade path, where the strictNodeInterop flag then only acts as the final "deprecation" step to align with Node interop.

That's what our esModuleInterop flag already does for babel's interop; but here's the thing - it's flagged because enabling that behavior has the potential to introduce types that won't exist in a user's compilation (and thus, confusion), eg, if some one were to import d from "foo", where foo is either a cjs or esm module (because some people are advocating for "dual mode" packages), depending on how the remainder of their build pipeline is setup, we have no idea which it'll be fulfilled by (especially if both are represented by the same declaration file, but its still ambiguous even if they're separate); and for one of those choices the import is likely invalid (esm), causing us to fail to issue an error on the bad import when we should have, and allowing a runtime error to occur (or the reverse is true - we forbid the import, but it would have worked at runtime under the config they were using and they're frustrated that they can't do what their docs say is right). Which is bad - when the typechecker is uncertain or fails to meet expectations, developers lose trust in it.

The point I've been trying to hammer home is that while, with explicit intent described for all inputs, yes, you can make any module system interop scheme your heart desires work out in the end; but absent intent (aka configuration), even the current light interop mechanism leads to ambiguity in the ecosystem. It would be far, far less error-prone if tools could consider esm and cjs as roughly interchangeable.

it seems like the summary of this is that people want an api to resolve and get the named exports of a file by node's loader's rules?

I'd find that useful for other reasons (reducing code duplication in the ecosystem by relying on a platform primitive), but isn't really a core point here.

@devsnek
Copy link
Member

devsnek commented May 12, 2018

but isn't really a core point here.

the only other thing i see here is ts devs trying to figure out some ts-specific detail of how to represent an cjs module wrapped with esm? maybe i'm wrong because these comments are so long and spaghetti but i can't seem to find any other point here. for those of us that don't use ts, etc, can someone here explain concisely how this relates to node's module implementation so we can help out?

@guybedford
Copy link
Contributor

eg, if some one were to import d from "foo", where foo is either a cjs or esm module (because some people are advocating for "dual mode" packages), depending on how the remainder of their build pipeline is setup, we have no idea which it'll be fulfilled by

I think for this reason it is important for the resolver to inform both the type checker and the emission what the module format is - by knowing if the dependency module is CommonJS or ESM it can do the right thing here. If it doesn't know anything about the dependency module, then I guess it could fall back to some default behaviour.

For example, already TypeScript can know that a .mjs file shouldn't do that special "default" esModuleInterop mode, but that a .js file in a CommonJS package should have an interop default.

The point I've been trying to hammer home is that while, with explicit intent described for all inputs, yes, you can make any module system interop scheme your heart desires work out in the end; but absent intent (aka configuration), even the current light interop mechanism leads to ambiguity in the ecosystem.

Completely agreed - and this is why I'm a firm believer in having the NodeJS interop as recommended by this group become that exact intent that tools can assume by default, then perhaps still have those configurations options to use other intentions for more loose interpretations.

It would be far, far less error-prone if tools could consider esm and cjs as roughly interchangeable.

The closest to interchangeable we've yet come through a proposal to the implementation that I know of would be for named exports iterated off of CommonJS modules available on the namespace. Even with that, ES modules wouldn't be able to be treated as their "default" export as CommonJS modules can be, an example of a difference that isn't possible to reconcile.

@DanielRosenwasser
Copy link
Member

Let's back up. My understanding is that right now we are trying to understand the use-cases, not solve them. It sounds like not everyone completely understands the scenario, so let me try to summarize the use-case so we can get everyone on the same page.

Let's say you have a CommonJS file like so:

// ./some-cjs.js
module.exports.foo = function() { /*...*/ };
module.exports.bar = function() { /*...*/ };

Babel and TypeScript users are currently able to import that file as follows:

import { foo, bar } from "./some-cjs";

foo();
bar();

TypeScript, being a JavaScript type-checker, also tries to ensure that users are correctly importing foo and bar.

However, rather than scanning the contents of the .js file, TypeScript will always prefer to look at what's called a .d.ts file, which describes the shape of a specific .js file. This is significantly cheaper, as no code needs to be checked, no inference needs to take place, and type metadata is immediately present.

Because of the ability to import using named imports, users will likely have authored the .d.ts file as follows:

// ./some-cjs.d.ts
export declare function foo(): void;
export declare function bar(): void;

Notice that the .d.ts file uses top-level ES exports. This implies that the JavaScript implementation could have been written as follows:

// ./hypothetical-module-shape.js
export function foo() {}
export function bar() {}

If the user in our original use-case (Rachel) switches her emit to target ES modules directly and rely on Node's ES module interop, her expectation is for her code to continue to function since the type-checker has given her some assurance.

@devsnek
Copy link
Member

devsnek commented May 12, 2018

@DanielRosenwasser thank you for keeping it concise 👍 however i'm still unsure of where the issue for node comes into play. is it the .js extension? i assume ts can output transpiled files with any extension right?

and more to my issue with this issue, why is a specific superset of javascript being considered for use cases? i would assume we want to make things best for people actually using node esm, as they actually have to deal with it. people who transpile can put any sort of magic between them and the system without worrying about it.

in my mind the above use case can be simplified to not include typescript or babel at all, but rather be the following:

Rachel is used to seeing the keys of the commonjs module.exports be used as named exports throughout the ecosystem. She wants to continue doing this with node's ESM loader.

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented May 12, 2018

in my mind the above use case can be simplified to not include typescript or babel at all, but rather be the following:

Rachel is used to seeing the keys of the commonjs module.exports be used as named exports throughout the ecosystem. She wants to continue doing this with node's ESM loader.

Yes, I agree, that's probably the most concise way to approach it. I think I was getting not just at the expectation, but why our user has that expectation. Existing tooling, infrastructure, and community has guided this user to write their code in a specific way with a strong degree of confidence. Maybe that wasn't necessary.

and more to my issue with this issue, why is a specific superset of javascript being considered for use cases? i would assume we want to make things best for people actually using node esm, as they actually have to deal with it. people who transpile can put any sort of magic between them and the system without worrying about it.

That's actually the thing - there's a limitation to the sort of magic you actually can achieve in this scenario. Throwing more tooling from the TypeScript/Babel/Flow side which requires users to reconfigure their builds isn't desirable either. But I guess we can circle back on this at a later date.

@devsnek
Copy link
Member

devsnek commented May 12, 2018

Throwing more tooling from the TypeScript side which requires users to reconfigure their builds isn't desirable either.

well they brought that on themselves didn't they? i'm sorry to be harsh and unpleasant but i think its rude to users of node.js to compromise their experience because typescript/babel users don't want another line of config. i firmly believe that node.js exists for everyone to use, but those who choose to place a layer of abstraction between it and them shouldn't be given priority.

@GeoffreyBooth
Copy link
Member

well they brought that on themselves didn’t they? i’m sorry to be harsh and unpleasant

Hey, can we please keep to a “yes, and” attitude here? TypeScript/Babel users are Node users. They deserve a good experience too. Let’s please not be dismissive of people’s feature requests.

@DanielRosenwasser
Copy link
Member

well they brought that on themselves didn't they?

Babel, TypeScript, and Flow users are Node users. They bring value to the ecosystem and telling them "you should have made better choices" or "tough luck" is not an ideal way to foster that community, especially when module interop has clearly been one of the hardest topics to reason about.

i think its rude to users of node.js to compromise their experience because typescript/babel users don't want another line of config.

I'm not sure what implied that I want to compromise the experience for Node.js users. I don't think this is a zero-sum game between Node users and compiled JS users. We're just discussing the use-case.

i'm sorry to be harsh and unpleasant

I get it, but I think we can work together here without dismissing each other's goals.

@weswigham
Copy link
Contributor Author

weswigham commented May 13, 2018

That "extra layer of abstraction" in this case happens to be a static analysis tool powering editors for people who may not be using any tools at all (from their PoV), tbf (and I keep saying that the typechecking is a sticker issue than the emit). That's the problem - if interop makes it so code can't be analyzed as it was before (or calls into question analysis validity due to ambiguity), or has the potential to change how existing code should be analyzed, that breaks that editing experience just like how changing how cjs would break the runtime experience.

well they brought that on themselves didn't they?

That comment is real easy to interpret as either tone deaf or victim shaming. But I'll assume that's not the case since we're all here to give input as various community stakeholders, who are participating to make sure their PoV is heard. Also, let me be straight here: nobody gets to develop node in isolation of node's community (AFAIK, nobody's just tossing a runtime over a fence and saying "have at it"). Remember the people who adopted all these tools to use modules were, yknow, excited about them. Burning that community probably isn't a desirable outcome, if it's avoidable, since they were the proving ground for the format...

@mcollina
Copy link
Member

A lot of people in this group are very supportive of transpilers users. However this is hard, mainly because the transpiled ESM syntax was made public with a very different semantic compared to the spec. Those users would be disappointed, independently of what we do (minus going against the spec).

I think it should be acceptable for those users to update their transpilers, and for the transpiler to emit code that is compatible with Node.js and how best we can support interop.

Unfortunately, supporting this case without a tooling update would mean to evaluate the JS file, which is not possible according to the spec.

@devsnek
Copy link
Member

devsnek commented May 13, 2018

i wasn't trying to say that they aren't valid users of node.js, i completely respect them as such. i was responding to the idea that (maybe) changing a part of our design is worth transpiler users not needing an extra bit of config, especially due to the context of this issue, which is hitting up against the safety of users without transpilers (see @mcollina's point about evaluation above). i chose not to mention this because i was trying to support the "yes, and" ideals without shooting down this use case while also voicing my frustration. i apologise that it came across the way it did.

@DanielRosenwasser
Copy link
Member

@mcollina @devsnek my understanding from #41 was that

people want to document use cases and also want to avoid speaking to implementation concerns

where implementation concerns include concerns relating to spec compliance. So I don't think we're looking for workarounds or community guidance, I think we're trying to just keep these users in mind so that we can guide our discussions to best serve those users.

@bmeck
Copy link
Member

bmeck commented May 14, 2018

I think there is some conflicting interests going on here where there is a desire to keep current implementations of compile to JS languages which may need to change their output depending on the Node implementation. I think the summary of the use case as:

Rachel is used to seeing the keys of the commonjs module.exports be used as named exports throughout the ecosystem. She wants to continue doing this with node's ESM loader.

Is in conflict with the phrasing of:

If the user in our original use-case (Rachel) switches her emit to target ES modules directly and rely on Node's ES module interop, her expectation is for her code to continue to function since the type-checker has given her some assurance.

Due to talking about 2 different code bases; the input codebase for the compile to JS language, and the output codebase. Since we are talking about Node's implementation in this working group we need to discuss if it is ok that input codebases and/or compilers for compile to JS languages might need to be altered by our implementation when those tools output ESM instead of CJS.

Note: I think this is a big topic to discuss and probably should be brought up outside of this specific use case, but wanted to note that the discussion seems to have some talking past each other due to conflicts. I'm not trying to cast an opinion.

@giltayar
Copy link

I believe there's a big elephant in the room, which we've been avoiding till now, and kudos to @devsnek for voicing it in the simplest way possible:

Rachel is used to seeing the keys of the commonjs module.exports be used as named exports throughout the ecosystem. She wants to continue doing this with node's ESM loader.

Unfortunately, to do that we need to break the ESM loader rule of not evaluating anything during the parsing phase. To figure out the keys of module.exports, we need to evaluate the module code during ESM's parsing phase.

And this is the conundrum we are in. We either:

  • Follow the ESM spec, and break Babel/TS users code that used named exports when importing CJS
    or...
  • Not follow the ESM spec, and please Babel/TS users who's code will be forward compatible

And it's much more than just expectations. There's a HUGE volume of code out there that follows this expectation, and is assuming that all will be well when it migrates to Node ESM.

Currently, NodeJS is on the "follow the ESM spec and break Babel/TS user expectations" side. And because of that, TS has the problem of the dual interpretation of CJS modules: do they interpret them as Babel would, or as Node ESM would.

As much as I hate revisiting decisions that were already made, I believe many in the group want to revisit this specific decision (no named imports for CJS), and rightly so (rightly so not because I agree, but because otherwise this group will endlessly be discussing it in one form or another).

I believe this is one of the more important decisions this group needs to make, and I believe we should make it as early as possible.

So let's discuss and get to an agreement on CJS named exports in one of the following group meetings, and if needed, revisit this TS issue afterwards.

@GeoffreyBooth
Copy link
Member

@giltayar Do you mind opening a new issue for this, rather than overloading this one? The parsing phase issue came up in #70 too and I think it deserves its own thread.

@weswigham
Copy link
Contributor Author

Soooooo... To walk all the way back to the issue from the discussion in #81, I'd like to remind everyone here that good transparent interop isn't just named imports and exports - it's capable substitutability: The ability to swap a cjs module out for an esm one or visa-vera and actually have a shot at preserving more of the same API under both import styles (so libraries can migrate without consumers needing to upgrade).

Named exports for cjs with import are part of that, but there's also more; like being able to fetch and evaluate esm synchronously with require, rather than asynchronously with import. Just a reminder that this usecase doesn't map to just one feature.

@bmeck
Copy link
Member

bmeck commented May 16, 2018

@weswigham @DanielRosenwasser

Is there any reason Typescript cannot compile to ESM that performs interop through a runtime like it currently does if those features need to continue? I'm still unsure on how this is about Node's implementation but it seems to be a use case requirement of Typescript's output to ESM needing to keep up their synthetic behaviors that currently are already being done. Some things seem to have problems being fixed at runtime like top level await explicitly not working with synchronous require and would probably have to require a full preprocessing step on an entire codebase and changing the behavior of require.

This issue was spawned with compatibility use cases needing to be preserved. If Typescript can output valid ESM that performs your synthetic behaviors properly as it already does today, what exactly is the use case that needs to be preserved by Node?

  • I see mention of type definition files using ESM-like syntax to define both ESM and CJS shapes for namespace, and module.exports but that doesn't appear to be a runtime use case that Node can provide. If there was a more concrete example of why Typescript cannot represent something except the current behavior of its synthetic ESM behaviors that would be helpful. Right now it just feels like the type definitions are somewhat unspecific shapes that apply to both CJS and typescript's output for ESM. What exactly needs to be done so that typescript can handle mapping other formats as they arise? Do these requirements apply to WASM / BinaryAST / HTML / etc. ideas of importing other formats?

    This is the point I am most confused on.

  • I see mention of named imports from CJS, but that seems to just be output for synthetic imports like Typescript does today. I'm not sure what the use case is in relation to Node for Typescript if it can generate output like it already does. It doesn't seem to be a use case on named exports itself and feels like it should be rephrased.

  • I see mention of synchronous require of ESM; that seems to be in conflict with what is in the TC39 pipeline, but does seem like something that cannot be solved with modified output from Typescript. I'm not entirely sure of what the use case of this is except behaving like Typescript, but it seems valid even if no solution is apparent.

@weswigham
Copy link
Contributor Author

Is there any reason Typescript cannot compile to ESM that performs interop through a runtime like it currently does if those features need to continue? I'm still unsure on how this is about Node's implementation but it seems to be a use case requirement of Typescript's output to ESM needing to keep up their synthetic behaviors that currently are already being done

There's no real emit issue. The issue is analysis. If node departs from the community, analysis of files that look the same can differ, heavily, based on the desired target runtime, which is the undesirable part, as the only "fix" is to manually specify what host you're targeting. You need to be aware that your esm flavor is just that - another flavor that we need to support, that is unless it's always subsitutable with either the downleveled esm flavor or the browser esm flavor (neither of which are the case in the current implementation). We need a separate set of logic for analyzing and checking the imports and exports in each flavor, irregardless of the emit. And if the flavor isn't determinable by syntax (and because everyone's repurposing the same syntax, it's not), we have no way to tell what a random module is in the absence of configuration, so need to go with our historical default (downleveled esm flavor). That's just an awful experience for anyone opening an editor on a node-esm project, as they're likely to be greeted with a bunch of erroneous squiggles.

I see mention of type definition files using ESM-like syntax to define both ESM and CJS shapes for namespace, and module.exports but that doesn't appear to be a runtime use case that Node can provide

We use the export namespace assignment as a syntactic marker that the file can most definitely only be represented at runtime by a cjs style module, it's fine. It's just that there's a lot of declarations that do not use that and just list named exports because the expectation is that the esm would be substitutable for the cjs.

@weswigham
Copy link
Contributor Author

weswigham commented May 16, 2018

I see mention of synchronous require of ESM; that seems to be in conflict with what is in the TC39 pipeline

Afaik the process is staged, but as yet there's nothing actually taking place across event loop turns? Only the dynamic import proposal's HostImportModuleDynamically and FinishDynamicImport addition even makes mention of a promise capability. I do understand that some future additions (ie, top-level await) could introduce changes here, though; but I do think those are still doable even if the general case is still handled synchronously.

@weswigham
Copy link
Contributor Author

Do these requirements apply to WASM / BinaryAST / HTML / etc. ideas of importing other formats?

We've always been under the assumption that, since they'd need to map to namespace objects, an esm module would be substitutable for them, as an esm module should be capable of producing any shape of namespace possible.

@bmeck
Copy link
Member

bmeck commented May 16, 2018

@weswigham

It's just that there's a lot of declarations that do not use that and just list named exports because the expectation is that the esm would be substitutable for the cjs.

What is the problem with typescript continuing to provide the runtime to support this appearance using the overloaded syntax as you point out? It does this today transforming CJS into ESM where CJS has a single module.exports value.

We've always been under the assumption that, since they'd need to map to namespace objects, an esm module would be substitutable for them, as an esm module should be capable of producing any shape of namespace possible.

So, what is the problem with the shape of the non-typescript like modules and why is it different from all those other types of modules? I don't understand. You still have the ability to parse import out of typescript files and declare interop strategies. You even mention configuration being applied already and a default configuration if none is supplied.

@weswigham
Copy link
Contributor Author

weswigham commented May 16, 2018

What is the problem with typescript continuing to provide the runtime to support this appearance using the overloaded syntax as you point out?

Under the current implementation there's no way to synchronously import native esm (without doing something like the esm loader does where you do on the fly runtime transpilation). So we can't. Plus, we know which declaration files are definitely cjs, but the rest could be anything (including still cjs) - that means with only the imported path and the object that results from somehow fetching that path, we need to map it back into the correct esm shape for that structure. For cjs that's would just br hoisting exported names if the runtimes doesn't provide that (with babel-style interop, anyway), but more formats have the potential to complicate that runtime helper greatly. We were actually looking at the code to polyfill wasm modules the other day - it's not particularly pretty (and you can only really support it synchronously in node (thanks to size limits in the browser), otherwise you need a before-app wasm preload step like webpack does); but at least they're easily substitutable for a normal esm module, so no shape mapping would need to happen.

So, what is the problem with the shape of the non-typescript like modules and why is it different from all those other types of modules? I don't understand.

The point is that any problem can be fixed with one more layer of abstraction (ie more configuration), but that doesn't make the fix a good user experience. We cannot support node's current esm implementation as a default behavior - it breaks far too much with what the community has expected esm should work alongside cjs, and if we applied logic that worked for the current node implementation to older code, we'd get spurious errors for bad imports, and going the other way, new node code loses safety on imports that shouldn't exist. It's another way to interpret the same code that is incompatible with the old way, irrespective of the runtime behavior - the analysis on its own is incompatible.

@bmeck
Copy link
Member

bmeck commented May 16, 2018

but that doesn't make the fix a good user experience

This might be my mismatch. I still don't map how this affects Node user experience, only the Typescript user experience. You even have discussed ways that Typescript could address this and make it hidden from the Typescript user.

@weswigham
Copy link
Contributor Author

weswigham commented May 16, 2018

@bmeck There's a huge base of node devs using JS in, eg, vscode. Our typechecker powers all of the editor features for JS there (also webstorm, if I'm not mistaken, and likely any other IDE that supports or has plugins to support the language server protocol). If the editor (ergo, us) can't determine what the JS is for, we can't determine the correct way to check it, thereby can't determine the correct, eg, import completions.

I know for some reason everybody likes to think of TypeScript as only a transpiler, and it certainly has some features for that, but it is far more used as a JS static analysis tool, because by nature it needs to be very good at that.

@bmeck
Copy link
Member

bmeck commented May 16, 2018

@weswigham I agree it is a large amount of users, but you have even made suggestions above on how to handle this.

@weswigham
Copy link
Contributor Author

weswigham commented May 16, 2018

If we're configured to do so, yes. But only if we're told to operate under the assumption we're targeting node. Even a package.json file isn't a good indicator of a project's nodiness, since it's used for frontend deps (or even transpiled deps that still need our current behavior), too.

And most people won't open their editor and configure it. A lot of people expect a functional out-of-the-box experience (we still occasionally get reports about how webpack does something a bit differently than we do and their code doesn't work and we have to tell them about a compiler flag to effectively tell the compiler "yes, I'm using webpack", because we can't just assume that kind of thing). Guessing at which configuration is right out of a suite of options does not help with that. And that doesn't even begin to touch on how sometimes someone with a browser/cjs targeted project might want a fraction of their codebase to work on both native esm and downleveled (like people have been advocating for dual-mode packages) - checking that is almost unreasonable, as it'd be the intersection of both formats, and is another format we'd have to guess at in the absence of explicit configuration.

TL;DR: Nobody configures anything if they can avoid it, people get angry are red squiggles or completions that are wrong and blame us, even if it's not our fault that there's 3 or 4 ways they could intend their project to be used and we had to guess at one of them.

@SMotaal
Copy link

SMotaal commented May 31, 2018

Being a huge of many aspects of TypeScript and the fact that it powers my editing experience, I must admit for me this past year when experimenting with ".mjs" files has made me question the status quo on many levels.

Since ".mjs" is not officially supported by TypeScript, those files started off looking as dim as plain text and gradually as Code and TS started adding features on par things got better.

My first take was it is TypeScript, then it is me, but ultimately it was everyone in the community. You can't expect every downstream tooling to have to reimplement core behaviours like static module resolution (both path and exported names) and expect everyone to design their ecosystem in such a way to afford the same flexibility for implementing such drastic changes.

We all worry when we see duplicate functionality across modules, so we really need to worry if how we resolve modules and describe namespaces gets done the same way. At any given moment, there are millions of NodeJS instances running where in the single instance you will find at least two or more packages (like Babel, CoffeeScript or TypeScript) resolving modules and describing namespace using their independent implementations which are at best functioning identically to the extent that they are configurable and configured to do so. I am not dwelling on performance much, simply on all the cost and penalties of redundancy.


If you want to unofficially test drive TypeScript with ".mjs" you can here

@ljharb
Copy link
Member

ljharb commented May 31, 2018

ESM is a new parsing mode - whether we can expect the community to update all its tools or not, that is a requirement regardless. Updating module resolution in the relatively small number of packages that control resolution is not unreasonable.

@SMotaal
Copy link

SMotaal commented May 31, 2018

@ljharb I completely agree, but where my thinking is going is more about the idea of a having some Module Resolution API which operates independently from the module loader, which can be consumed by the module loader and others, where all the side-effects of resolution-specific configuration can be harmonized. Aside from resolving the same thing, a plus would be not resolving the thing over and over again. Assume of course such an API is designed to accommodate different resolution intents.

@ljharb
Copy link
Member

ljharb commented May 31, 2018

@SMotaal that already exists for CJS - https://npmjs.com/resolve - and i think it should be left up to userland to do the same for ESM.

@SMotaal
Copy link

SMotaal commented May 31, 2018

I think I can better elaborate my view a little, it is not about having a golden standard replicate of Loader.resolve(specifier, referrer, …) but more about designing the module system in such a way that it can decouple loading from resolution to a point that would make it possible for all consumers to benefit from the optimized C++ resolution capabilities, it being speed or Node-specific resolution logic.

As far as both aspects are concerned, resolve is an excellent example of what will potentially be far more challenging with the new Loader. If performance is already a concern with experimental-modules, I can only imagine there will be little room to not compromise when creating a pure JavaScript replication (irrespective of prior art which preceded and was not intended to mimic Node's own implementation). If there is no deliberate effort to address this, people may even be tempted to actually create additional instances of Node's Loader or reimplement ModuleWrap in a native module, all of which will be very counterproductive to the gains from ModuleWrap.

Things that can be achieved from my perspective:

  • Loader's resolver can be forked
  • Forked resolver can match or diverge from parent resolver
  • Forked resolver can independently operate (safeguard's Loader)
  • Resolver takes care of package.json implications, including hooks
  • Resolver can use profiles: node, browser... etc
  • Resolver can be constructed irrespective of Loader or ESM
  • Resolver can provide metadata and/or static reflection (when possible)

@bmeck
Copy link
Member

bmeck commented May 31, 2018

Currently the Loader's main hook is resolve(...) and I think everyone wants a different solution to dynamicInstantiate. If the resolve hook is the only hook remaining, I think this doesn't appear to be far from your desires @SMotaal . I'm not sure what safeguard's loader refers to though.

@SMotaal
Copy link

SMotaal commented May 31, 2018

@bmeck people would want a Resolver for one of two reasons. The first would be to power a design or build time tool, ie this use case. The second would be at runtime time, I am thinking debugging or reflective aims, where historically hooking was accomplished by monkey-patching Module.prototype. I assume that the Loader.resolve idea tried to prevent such outcomes.

Consider a Loader.resolver.fork() instance, which would be used to resolve for purposes other than Loader.import, and this could have some effect of the state of the resolver. Safeguard would be that the forked resolver will be able to rely on the parent resolver's previous work without affecting it's state, and when the parent does not have records for a particular resolution, the forked resolver handles the request and stores the resulting records privately without affecting the parent or any of it's other children.

@bmeck
Copy link
Member

bmeck commented May 31, 2018

@SMotaal Can you clarify how the hooks cannot be reused ahead of time? I understand the lack of Loader.prototype.import use case for ahead of time work, but am unclear on why it needs to be separate from the hook specification a Loader's use. A lot of my push back on dynamically mutating loaders or the loader chain such as .fork() is around trying to make it isolated enough to get static / ahead of time tooling to be able to use these hooks.

I still don't fully understand Safeguard. Loader hooks intentionally don't have the ability to mutate the module map of the Loader instance using them.

@SMotaal
Copy link

SMotaal commented May 31, 2018

My thinking is for all runtime uses that are independent from the actual runtime loading of modules. A good example would be debugging or testing, where you do not want any resolutions that are strictly not runtime to have an effect on the runtime and with that the state of the loader. What I meant by safeguard is that if these extra-runtime applications need to do more resolutions, I am assuming and I can be dead wrong that this will result in some records in some map. If that is the case, a forked resolver (intended for things other than loading by the runtime loader's instance) would be a good place to store such records or not but absolutely not have those in the runtime loader's state.

@SMotaal
Copy link

SMotaal commented May 31, 2018

@bmeck trust me, I have grown to be a big subscriber to your "push back on dynamically mutating" which obviously was not how I felt about it around the same time last year. I am actually interested in doubling down on this idea which can only be possible by offering abstractions that empower users to accomplish things in a controlled way.

@MylesBorins
Copy link
Contributor

Can this be closed?

@weswigham
Copy link
Contributor Author

FWIW it's a discussion thread so it can always be reopened or forked if there's more discussion requested. IIRC I only opened this because during a meeting someone asked for some details on our PoV, so I wrote this up. There's no conclusions or further work or really anything actionable in this comment thread.

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

No branches or pull requests