Open bmeck opened 4 years ago
For upgrade when support is uncertain: I believe that we should be thinking about the gradual upgrade case in a way similar to async/await: use transpilers or bundlers if you are unsure of support.
For fallback: I think this is a broader problem for modules. Import maps had one solution, which I proposed building on for other kinds of conditionals. Now that import maps has removed fallback lists, I don't know what other kind of conditional would make sense to use. But I feel like this should be solved at another layer, or possibly with some particular use of module attributes with richer object values, otherwise it wouldn't be general enough for the cases where the need comes up.
It's also possible to use top-level await for importing modules and doing a fallback action if the import fails. However, this is best done sparingly, as it reduces the parallelism of the imports. That's why I wrote up the idea of import-map-feature-tests...
I don't think this feature request is something to block module attributes as a whole or JSON modules on.
My concern is more that I have a JSON module and want to convert it to WASM, so I want consumers of my module (including myself) to have a transition period where either is acceptable at the same URL. I'm a bit unclear on the fallback suggestion as saving an error on import failing would cause fallback on the same URL to also fail if the cache is shared.
I agree this isn't a blocker.
To permit a transition, you could wrap any module type in a JS module that does export * from "module" with type: "foo"
. Do you think this would provide a reasonable transition path?
@littledan that has a problem since the whole discussion was about loading JS being a problem for some viewpoints on web regarding JSON/WASM. Loading the seen as problematic JS seems to go against that concern.
Aside: Also the export *
form isn't a direct alias and won't bring in default
nor have object identity if we ever use module namespace object references for APIs (some people have talked about this recently).
Additionally, i don’t have the “security” concerns that motivate this entire proposal; I’d want to import from modules and always allow a type of JS or json, indiscriminately, added with a build process for the only environments where this matters, browsers and deno. How could i do that?
Sorry, I forgot about default... Really, I think this is an important problem, and I wanted to solve it with fallback lists, and I'm not sure what to do with the current state of import maps. This could be with either in-band or out-of-band indications.
import "//test.local/config" with type: "json", fallback: { specifier: "//test.local/config", with: { type: "wasm" } }
(I think it's pretty important to be able to select a different specifiers in this kind of fallback case, which is why I have it duplicated, but you could also imagine making the other design choice, while still using something based on module attributes and compound literals.)
tl;dr I like to think that module attributes is a step in a potentially useful direction for the problem you raise here, even if I'm proposing that it be excluded from the MVP, and I'm glad we can think these things through before proceeding.
Out of band would likely solve it either an array of fallbacks, just like import maps did at one time. Could the inline type also be an array of types?
@ljharb Well, sure, but I hope we have some way to give each one module attributes as well. So, it could be like this:
import "//test.local/config" with type: "json",
fallbacks: [{ specifier: "//test.local/config", with: { type: "wasm" } }]
Or, sure, we could have an array of types., like import "//test.local/config" with type: ["json", "wasm"]
. However, I'm pretty convinced that you'd want to be able to change the URL when falling back sometimes, so I'm pretty skeptical of going that way.
I’d say those kinds of complex cases could use dynamic import or import maps without much difficulty.
@ljharb This tradeoff on what we want to support with built-in fallback mechanisms and what we want to leave to dynamic import/TLA is a tricky. I think we could use some more investigation into use cases to draw this line. I'm proposing that we leave that investigation to a follow-on proposal, knowing that we're deliberately leaving space for that proposal to work. Does this seem OK?
@littledan that seems fine, but might require some sort of note about any cache invalidation that a trade-off might perform in the context of a shared URL.
@bmeck The current normative text in https://tc39.es/proposal-module-attributes/#sec-hostresolveimportedmodule is,
Each time this operation is called with a specific referencingScriptOrModule, specifier, otherAttributes tuple as arguments, if otherAttributes differs from attributes only based on the value of type it must return the same Module Record instance if it completes normally.
"if it completes normally" allows for exceptions to be thrown for one type without that being part of the cached value. This would allow for importing with one type to fail, but another type to succeed. I intended this to be useful without fallbacks in mind, but I imagine that this would also suit the requirements here, for type fallbacks. Do you think it does?
@littledan does that mean you can retry with the same type (currently not possible for dynamic import)?
With import()
as specified, I believe you can already retry infinite times until the module record completes normally.
@bmeck Yes: the host is allowed to choose whether to throw an exception or complete normally (with a single value) based on the type.
that is probably fine for now, we can leave this to a follow on
I think it's worth considering the fallback approach in this proposal for those that want to write universal code but don't care about the "security" concerns motivating the proposal.
I agree with @littledan about the importance of supporting different specifiers (and IMO specifier reuse will be the rare case) while not losing the ability to recognize fallbacks as defining a single import. And if this is to be solved in-band, then it would be nice for the syntax to reflect a coupling between specifier and module attributes, e.g.
import '//test.local/config' with { type: "wasm" },
'//test.local/config' with { type: "json" }
(which should work because ,
is not currently valid syntax after an automatically-inserted semicolon AFAIK).
Further, keeping in mind the non-trivial penalty for iterating through a fallback list, we might add a module attribute to express conditions like those of import-map-feature-tests:
import '/empty.mjs' with { condition: { "validSyntax": "0m" } },
'/polyfills/BigDecimal.mjs'
Both of these are worth thinking about now, but can be deferred to future extensions (especially if the attributes are delimited and unknown attributes are ignored, cf. https://github.com/tc39/proposal-module-attributes/issues/21#issuecomment-551987820 ).
@gibson042 Could you explain why you prefer that additional syntax to the suggestion I made in https://github.com/tc39/proposal-module-attributes/issues/56#issuecomment-628064562 ? Also, do you agree with my classification of this issue as something that could be solved in a follow-on proposal?
I prefer fallbacks to use the same syntax as the primary import, which avoids introducing another layer of nesting and two special concept names (fallbacks
and specifier
) and divergent syntax (for the fallback specifier
and with
).
-import "//test.local/config" with type: "json",
+import "//test.local/config" with { type: "json" },
- fallbacks: [{ specifier: "//test.local/config", with: { type: "wasm" } }]
+ "//test.local/config" with { type: "wasm" }
But to reiterate, yes, this could be solved later as long as this proposal requires host-unknown attributes to be ignored.
Do we have concerns about module upgrade paths where you are intending to transition from 1 type to another and/or are not concerned with the target type?
etc.