tc39 / proposal-import-attributes

Proposal for syntax to import ES modules with assertions
https://tc39.es/proposal-import-attributes/
Apache License 2.0
599 stars 26 forks source link

Allow assertions to affect module loading, change keyword to `with` #131

Closed nicolo-ribaudo closed 1 year ago

nicolo-ribaudo commented 1 year ago

This PR is based on the outcome of the calls of the modules group in the past weeks.

PREVIEW: https://nicolo-ribaudo.github.io/proposal-import-assertions/

There are three main parts:

When discussing about this proposal, we are now referring to it as "attributes" instead of "assertions". This PR doesn't change that yet, and instead only focuses on semantics (since it's just a communication/editorial change).

We also discussed about suggesting tools to not use import assertions unless they are explicitly prefixed and marked as "this only works in my tool" so that users don't expect them to be portable (for example, _webpackLoader: "css"). I'm not sure about where/how to write that.

cc @guybedford @jridgewell @littledan

ljharb commented 1 year ago

When you say "Throw on unsupported assertions", obviously this is what import() would have to do - but are unsupported attributes an early error in static import?

GeoffreyBooth commented 1 year ago

If the runtime throws on unsupported attributes, how would new attribute keys ever be able to be added? I would think that for browsers this would mean that whatever attribute keys are supported when this syntax initially ships would be the only keys ever supported, as no one would be able to use a new key without worrying that some older browser that doesn’t support the key would throw on encountering it. (Or there would be a lag time of years for each new attribute key added, so that authors are confident that older browsers are disused enough that the new attribute is supported by all the browsers they care about.) Likewise with libraries written for Node, where library authors would have to worry about older versions of Node throwing on an unsupported key.

I’d also like to hear from the browser folks how the HTML spec plans to support various attribute keys, such as type and whatever else comes later. Currently type is part of the module map key; would every other supported key also become part of the cache key? When implementing import assertions for Node, I found that I needed to follow how the HTML spec handled module map cache keys in order to provide consistent behavior with how Node and browsers each handle dynamic import() race conditions; I raised this in https://github.com/tc39/proposal-import-assertions/issues/114. This is also something that could or arguably should be standardized here, rather than in the HTML spec that server-side runtimes would also need follow in order to provide consistent behavior with browsers.

justinfagnani commented 1 year ago

how would new attribute keys ever be able to be added?

The same way as any new syntax, right? Compilers will have to downlevel new keys and new values for older browsers.

ljharb commented 1 year ago

This is syntax, and any new syntax can't generally be added in a nonbreaking way. Things that aren't supported shouldn't parse.

GeoffreyBooth commented 1 year ago

The same way as any new syntax, right?

The previous syntax, and as far as I'm aware this one too, treats the keys in the assertion “object” as user-defined strings; they're not keywords. As such I'd expect to be able to add other keys. I thought that was the point of using the object form.

justinfagnani commented 1 year ago

Attribute values seem to be very much like import specifiers. They look like strings, but they are values that different hosts do and do not support, and if a value isn't supported there's an error. Toolchains and import maps deal with translating values to supported forms.

edit: and in the case of import() they really are strings, and tool support for transformations is just more complicated - they don't support dynamic values, etc.

GeoffreyBooth commented 1 year ago

Attribute values seem to be very much like import specifiers. They look like strings,

I’m asking about keys, not values. From https://github.com/tc39/proposal-import-assertions/#proposed-syntax:

Here, a key-value syntax is used, with the key type used as an example indicating the module type. Such key-value syntax can be used in various different contexts.

And in this PR, in https://github.com/tc39/proposal-import-assertions/pull/131/files#diff-7d681727fcf47dc0b9a7512a470fb0da63276c625891a5cc232d725bd12912fdR139:

1. Append the ImportAssertion Record { [[Key]]: _key_, [[Value]]: _value_ } to _assertions_.

If the plan is for runtimes to throw on any unrecognized keys (and values?), I guess that’s an option, but it’s a significant change from the previous Stage 3 proposal. It would seem to significantly slow the introduction of new keys and/or values. Ideally there would be some way to gracefully fall back. Maybe that’s just dynamic import() and try/catch? Presumably the error is a runtime one and not a parse error?

I would also appreciate an answer about whether type will continue to be part of the module map for browsers (and by implication, everyone) and if that means we would be following that pattern for any other new keys that follow.

ljharb commented 1 year ago

@GeoffreyBooth imo the point of the object form is that it's expandable in the future without additional grammar changes - but only TC39 would be adding new ones, not individual hosts or tools.

nicolo-ribaudo commented 1 year ago

@ljharb When you say "Throw on unsupported assertions", obviously this is what import() would have to do - but are unsupported attributes an early error in static import?

Yes, it's an early SyntaxError, defined at https://github.com/nicolo-ribaudo/proposal-import-assertions/blob/f5e05308608689068c48066bda6861d94f0ce34e/spec.emu#L682. For dynamic import, it's a TypeError, defined at https://github.com/nicolo-ribaudo/proposal-import-assertions/blob/f5e05308608689068c48066bda6861d94f0ce34e/spec.emu#L141.

@GeoffreyBooth If the runtime throws on unsupported attributes, how would new attribute keys ever be able to be added? I would think that for browsers this would mean that whatever attribute keys are supported when this syntax initially ships would be the only keys ever supported, as no one would be able to use a new key without worrying that some older browser that doesn’t support the key would throw on encountering it. (Or there would be a lag time of years for each new attribute key added, so that authors are confident that older browsers are disused enough that the new attribute is supported by all the browsers they care about.) Likewise with libraries written for Node, where library authors would have to worry about older versions of Node throwing on an unsupported key.

We have thought about this for a while, and there are three options for unknown keys:

Ignoring them was certainly the best choice with the current version of the proposal, where these modifiers are really just assertions: ignoring a (passing) assertion doesn't change the behavior of the code, so unknown assertions would have also worked in older engines.

However, now that these modifiers can affect how a module is evaluated, it's not usually a valid fallback mechanism anymore. Consider a few hypothetical examples:

All these example attributes cannot be simply ignored:

On the other hand, there might also be attributes that could safely be ignored. However, I couldn't find examples compelling enough to justify not throwing given all the cases where throwing would be better.

@GeoffreyBooth I’d also like to hear from the browser folks how the HTML spec plans to support various attribute keys, such as type and whatever else comes later. Currently type is part of the module map key; would every other supported key also become part of the cache key?

Yes, type will continue to be part of the module map for browsers. Part of the motivation for the changes to this proposal is that the current version recommends to not use assertions as part of the module map keys, but HTML (and by extension, everyone who is trying to match HTML) ignores that recommendation.

For other keys, it depends. For example, a possible resolutionMode key would need to be part of the module map key (import "mod" with { resolutionMode: "node_modules" } and import "mod" with { resolutionMode: "importmap" } may resolve to different modules), while a possible noHTTPImports key doesn't (once the first between import "mod" and import "mod" with { noHTTPImports: "true" } resolves, they would always both resolve to the same module).

This proposal leaves the choice to hosts. The way it's specified, from ECMA-262's point of view all the assertions are part of the module map key, but hosts are free to have the same module corresponding to different keys (same as how "./dep" and "../dir/dep"` can be mapped to the same module). Implementation-wise, I would expect the module map to only be actually keyed on the attributes that matter.

@ljharb imo the point of the object form is that it's expandable in the future without additional grammar changes - but only TC39 would be adding new ones, not individual hosts or tools.

Note that this PR doesn't change this yet, and it keeps the original "host gives to ECMA-262 the list of attributes it supports" strategy. There are some discussions about how to potentially restrict this, while still acknowledging that:

gibson042 commented 1 year ago

@GeoffreyBooth If the runtime throws on unsupported attributes, how would new attribute keys ever be able to be added? I would think that for browsers this would mean that whatever attribute keys are supported when this syntax initially ships would be the only keys ever supported, as no one would be able to use a new key without worrying that some older browser that doesn’t support the key would throw on encountering it. (Or there would be a lag time of years for each new attribute key added, so that authors are confident that older browsers are disused enough that the new attribute is supported by all the browsers they care about.) Likewise with libraries written for Node, where library authors would have to worry about older versions of Node throwing on an unsupported key.

We have thought about this for a while, and there are three options for unknown keys:

  • ignore (as in the current version of the proposal)
  • pass everything to the host, and let the host decide if it should throw or ignore
  • throw

Ignoring them was certainly the best choice with the current version of the proposal, where these modifiers are really just assertions: ignoring a (passing) assertion doesn't change the behavior of the code, so unknown assertions would have also worked in older engines.

However, now that these modifiers can affect how a module is evaluated, it's not usually a valid fallback mechanism anymore.

I think this is glossing over an upgrade concern that was deferrable on the basis of unknown attributes being ignored (cf. https://github.com/tc39/proposal-import-assertions/issues/21#issuecomment-551987820 and https://github.com/tc39/proposal-import-assertions/issues/56#issuecomment-629698719 ). Will authors be required to split their source files in order to leverage new attributes in static imports without breaking old (or even not-so-old-but-not-bleeding-edge) engines? I guess so, but that implies an outer mechanism like import maps (and tooling to leverage it, especially regarding deep imports) that isn't available to an arbitrary ECMAScript implementation.

Consider a few hypothetical examples:

  • import "mod" with { resolutionMode: "importmap" }, which would make Node.js resolve "mod" from an external import_map.json file (like HTML and Deno) instead of using the default node_modules-based resolution algorithm
  • import "./image.png" with { type: "image", as: "arraybuffer" }, import "./image.png" with { type: "image", as: "dom" }, import "./image.png" with { type: "image", as: "canvas" }, which would allow browsers to import images respectively as ArrayBuffer, HTMLImageElement, and CanvasRenderingContext2D objects (with one of them being the default, assuming that type gets implemented before as).
  • import styles from "./main.css" with { type: "css", layer: "utilities" }, that would allow importing a specific CSS layer, similar to @import "./main.css" layer(utilities).

Thank you for the examples. I think the middle one might be better spelled with distinct type values like "image-arraybuffer" vs. "image-dom" vs. "image-canvas" since the resulting imports would have different shapes, and I'm not sure that old engines refusing to load the third example (rather than ignoring layer and executing later feature-detection-dependent fixup) is a good thing, but the first example does seem salient.

On the other hand, there might also be attributes that could safely be ignored. However, I couldn't find examples compelling enough to justify not throwing given all the cases where throwing would be better.

What about building in an escape hatch? If the import itself has a means of expressing attribute-level ignorable/required status (subject to host approval, e.g. type is never ignorable), then we get behavior aligned with anticipated predominant use cases but avoid precluding future alternatives.

There's recent if somewhat distant precedent for a similar concept affecting RFC 3339 timestamp suffixes, in which a leading ! opts in to criticality such that 2022-07-08T00:14:07+01:00[knort=blargel] may be accepted while 2022-07-08T00:14:07+01:00[!knort=blargel] is rejected.

justinfagnani commented 1 year ago

@gibson042

I think the middle one might be better spelled with distinct type values like "image-arraybuffer" vs. "image-dom" vs. "image-canvas" since the resulting imports would have different shapes

This point might need clarification with the update. With assertions my understanding is that type did not change the shape of the imported module - that was determined solely by the mime-type in browsers and extension in Node - it only asserted that the file type matched the import type.

That changes a bit if HTML uses type to send a header, but it doesn't change the fact that type: 'css' always expected the file to be CSS, or that type: 'image' would always expect the file to be an image, regardless of whatever form that was transformed into.

ie, for CSS you might have, {type: 'css', as: 'text'} to say that you require that you load an actual CSS file, but you don't want it parsed into a stylesheet.

nicolo-ribaudo commented 1 year ago

@gibson042 I think this is glossing over an upgrade concern that was deferrable on the basis of unknown attributes being ignored (cf. https://github.com/tc39/proposal-import-assertions/issues/21#issuecomment-551987820 and https://github.com/tc39/proposal-import-assertions/issues/56#issuecomment-629698719 ). Will authors be required to split their source files in order to leverage new attributes in static imports without breaking old (or even not-so-old-but-not-bleeding-edge) engines? I guess so, but that implies an outer mechanism like import maps (and tooling to leverage it, especially regarding deep imports) that isn't available to an arbitrary ECMAScript implementation.

Yes, if authors want to use static imports with attributes that are supported only in new versions of hosts they would need to provide two copies of the source files. This is annoying, but it's also how every modules-related feature has worked so far. Moreover, thanks to TLA, you can try/catch at the top-level around a dynamic import with the attributes you want. This is not effectively the same as a static import (since it changes the load/evaluation order of the graph), but for most users it's a good enough approximation.

I know that "one day we will be able to polyfill this" is not a good argument for a feature (so I don't consider this as a point in favor of this design!), but the ideal future I see is to be able to virtualize and polyfill any import attribute using the compartments proposal, with something like https://github.com/tc39/proposal-compartments/issues/37#issuecomment-1186141222.

@gibson042 I'm not sure that old engines refusing to load the third example (rather than ignoring layer and executing later feature-detection-dependent fixup) is a good thing

Regarding any attribute that can be ignored and that is possible to later fix after feature detection, I'm not convinced that

import _styles from "./main.css" with { type: "css", layer: "utilities" };
const styles = layerAttributeIsIgnored ? getLayer(styles, "utilities") : _styles;

is in any way better than just not using the attribute until all the runtime you care about support it, and instead always manually applying its equivalent semantics:

import _styles from "./main.css" with { type: "css" };
const styles = getLayer(styles, "utilities");

If an attribute can be "simulated", you can just always simulate it (which is actually similar to what happens when transpiling syntax: you transpile even for modern engines, until you don't care anymore about the older ones).

@gibson042 What about building in an escape hatch? If the import itself has a means of expressing attribute-level ignorable/required status (subject to host approval, e.g. type is never ignorable), then we get behavior aligned with anticipated predominant use cases but avoid precluding future alternatives. There's recent if somewhat distant precedent for a similar concept affecting RFC 3339 timestamp suffixes, in which a leading ! opts in to criticality such that 2022-07-08T00:14:07+01:00[knort=blargel] may be accepted while 2022-07-08T00:14:07+01:00[!knort=blargel] is rejected.

This is interesting, I wonder if it would be better to have it syntactically (with { layer?: "utilities" }, where the syntax is (Identifier | StringLiteral) `?`? `:` StringLiteral) or as part of the key itself just as a convention between hosts (with { "layer?": "utilities" }, where the syntax is (Identifier | StringLiteral) `:` StringLiteral). When I'll present this PR I'll mention that one of the design points we are reconsidering is about handling of unknown keys, and I'd be grateful if you (or someone else for you) could bring this up.

I'm curious how you think it could work "subject to host approval, e.g. type is never ignorable". If the list of ignorable (or not ignorable) keys are provided by the host, how would it work with future currently-unknown keys? If we hosts didn't introduce type now but in a few years, and they didn't know now that they would want to introduce type in the future, how could they already specify that it's not ignorable?

@justinfagnani This point might need clarification with the update. With assertions my understanding is that type did not change the shape of the imported module - that was determined solely by the mime-type in browsers and extension in Node - it only asserted that the file type matched the import type.

This is my understanding too, and that's still how it will be implemented in HTML. I assume for Node.js will be similar, and with { type: 'json' } will not allow importing a JS file parsing it as if it was JSON.

gibson042 commented 1 year ago

Regarding any attribute that can be ignored and that is possible to later fix after feature detection, I'm not convinced that

import _styles from "./main.css" with { type: "css", layer: "utilities" };
const styles = layerAttributeIsIgnored ? getLayer(styles, "utilities") : _styles;

is in any way better than just not using the attribute until all the runtime you care about support it, and instead always manually applying its equivalent semantics

It can definitely be better if the fixup is expensive or imperfect. Regardless, I don't want to belabor this point.

This is interesting, I wonder if it would be better to have it syntactically (with { layer?: "utilities" }, where the syntax is (Identifier | StringLiteral) `?`? `:` StringLiteral) or as part of the key itself just as a convention between hosts (with { "layer?": "utilities" }, where the syntax is (Identifier | StringLiteral) `:` StringLiteral).

Probably the latter, so it can be expressed for dynamic import in the same way—but ideally as part of the proposal itself rather than as an external convention.

When I'll present this PR I'll mention that one of the design points we are reconsidering is about handling of unknown keys, and I'd be grateful if you (or someone else for you) could bring this up.

:+1:

I'm curious how you think it could work "subject to host approval, e.g. type is never ignorable". If the list of ignorable (or not ignorable) keys are provided by the host, how would it work with future currently-unknown keys? If we hosts didn't introduce type now but in a few years, and they didn't know now that they would want to introduce type in the future, how could they already specify that it's not ignorable?

Having considered this further, I think the caveat might be unnecessary because any given attribute name is either known to an implementation (in which case it wouldn't be ignored) or unknown (in which case it couldn't be included in a list of non-ignorable attributes). type being in the initial proposal means that every implementation supports it so requesting ignore-if-unknown behavior is irrelevant (it's never unknown). Using your above examples, import "./image.png" with { type: "image", "as?": "arraybuffer" } would be treated like with { type: "image" } in old hosts and { type: "image", as: "arraybuffer" } in new ones, and any situation for which such variance is unacceptable would not include the ?.

An alternative design could involve consideration of not just the attribute name but also the value, such that a new implementation might want to reject import "./image.png" with { type: "image", "as?": "not-yet-shipped-format" } while an old implementation that doesn't even know about as would treat the import like with { type: "image" }. But that kind of variance doesn't seem helpful in either the name-only or the name-and-value design anyway. It's worth noting that neither design is fully general with respect to fallback, but I'm just striving for a bridge to something better while the range of valid values for any particular attribute has not been subject to many changes and thus that lack of generality isn't detrimental.

nicolo-ribaudo commented 1 year ago

I'm merging this PR now to have the proposal ready for plenary next week.

@gibson042 I'm happy to continue discussing that topic in an issue!

gibson042 commented 1 year ago

@nicolo-ribaudo #132