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

Multi-typed modules #56

Open
bmeck opened this issue May 13, 2020 · 21 comments
Open

Multi-typed modules #56

bmeck opened this issue May 13, 2020 · 21 comments

Comments

@bmeck
Copy link
Member

bmeck commented May 13, 2020

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?

import '//test.local/config' with type="json" || type="wasm"

etc.

@littledan
Copy link
Member

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.

@bmeck
Copy link
Member Author

bmeck commented May 13, 2020

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.

@littledan
Copy link
Member

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?

@bmeck
Copy link
Member Author

bmeck commented May 13, 2020

@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).

@ljharb
Copy link
Member

ljharb commented May 13, 2020

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?

@littledan
Copy link
Member

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.

  • If it's out-of-band, I'd suggest it look something like https://github.com/littledan/import-map-feature-tests/ modulo bikeshedding and making it not be the import map as such.
  • If we want to go with in-band, I wonder if we could do it using module attributes, with complex constant object values. The idea of compound constant literals could be defined with static semantics that check for this notion of constant-ness (though there are many details to bikeshed over). I'd suggest something like this:
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.

@ljharb
Copy link
Member

ljharb commented May 13, 2020

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?

@littledan
Copy link
Member

@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" } }]

@littledan
Copy link
Member

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.

@ljharb
Copy link
Member

ljharb commented May 13, 2020

I’d say those kinds of complex cases could use dynamic import or import maps without much difficulty.

@littledan
Copy link
Member

@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?

@bmeck
Copy link
Member Author

bmeck commented May 13, 2020

@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.

@littledan
Copy link
Member

@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?

@bmeck
Copy link
Member Author

bmeck commented May 13, 2020

@littledan does that mean you can retry with the same type (currently not possible for dynamic import)?

@ljharb
Copy link
Member

ljharb commented May 14, 2020

With import() as specified, I believe you can already retry infinite times until the module record completes normally.

@littledan
Copy link
Member

@bmeck Yes: the host is allowed to choose whether to throw an exception or complete normally (with a single value) based on the type.

@bmeck
Copy link
Member Author

bmeck commented May 14, 2020

that is probably fine for now, we can leave this to a follow on

@ljharb
Copy link
Member

ljharb commented May 15, 2020

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.

@gibson042
Copy link

gibson042 commented May 16, 2020

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. #21 (comment) ).

@littledan
Copy link
Member

@gibson042 Could you explain why you prefer that additional syntax to the suggestion I made in #56 (comment) ? Also, do you agree with my classification of this issue as something that could be solved in a follow-on proposal?

@gibson042
Copy link

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.

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

No branches or pull requests

4 participants