Repository navigation
Import assertions with loaders #39
Copy link
Copy link
Open
Description
Here's a draft of what solution I can think of regarding relations between loaders and import assertions.
Relation of ESMLoader class with import assertions:
Before resolve() |
resolve() |
Between resolve() and load() |
load() |
After load() |
|---|---|---|---|---|
Check if importAssertions.type value is supported |
nothing | use importAssertions.type to check for race conditions |
nothing | check if finalFormat is compatible with importAssertions.type |
| nothing | pass importAssertions to resolve() |
check if importAssertions.type value is supported, and use it to check for race conditions |
nothing | check if finalFormat is compatible with importAssertions.type |
| nothing | pass importAssertions to resolve() |
disallow race conditions?1 | nothing | check if finalFormat is compatible with importAssertions.type |
| nothing | pass importAssertions to resolve() |
one of the above | pass importAssertions to load() |
check if importAssertions.type value is supported and compatible with finalFormat |
| nothing | pass importAssertions to resolve() |
one of the above | pass importAssertions to load() and let load() do the checks |
nothing |
| nothing | nothing | use importAssertions.type to check for race conditions |
pass importAssertions to load() and let load() do the checks |
nothing |
| nothing | nothing | nothing | pass importAssertions to load() and let load() do the checks |
nothing |
- The first one is the approach on my first
Unflag import assertionsPR, drawbacks of this approach have been discussed. - The second seems really appealing to me, it would allow folks to write hooks to support assertion-less JSON, and also support import assertions that are not supported by Node.js.
- Same thing except the loader throws if a URL is loaded with different assertions. It puts the burden of handling race conditions to the
resolvehook. Not sure this is actually doable and/or desirable. - pass the output
importAssertionsofresolve()toload()context. This adds complexity, not sure it's worth it. - Same but now the loader hook is in charge of doing the verification, so users can opt-out. That seems... dangerous? but maybe it's actually OK?
- Same but import assertions are not passed to
resolve().
My (new) favourite is 2.: it seems to me like the perfect balance between simplicity and letting users do powerful things (e.g.: it would be quite easy to implement a loader that supports assertionless imports). I have a PR almost ready implementing this.
Anyway, we can discuss that on next week meeting since Goeffrey invited me, but happy to hear thoughts before that if people want to share them.
Footnotes
-
if the resolver returns an
urlthat's already in the cache and the returnedimportAssertion.typehas a value different from the one in the cache, throw aTypeError. ↩
Metadata
Metadata
Assignees
Labels
No labels
Activity
GeoffreyBooth commentedon Oct 6, 2021
Is this table for the current nodejs/node#40250 PR, or a potential future where the assertions are optional?
I don’t think we should be doing much, if anything, between
resolveandload. As a loader author, I would expect the return value ofresolveto be passed intoloadwithout much happening in between.I’m going to take a stab at a thinking-out-load proposal; feel free to point out its flaws or suggest a better alternative. Take as an example a YAML loader, to make possible code like this:
For this case, the YAML loader’s
loadhook would be where we assert that the data is actually YAML before converting it to JSON and returning{ source: convertedData, format: 'json' }, and then thejsontranslator would take it from there. Node would not do any validation of the assertion; the validation would have to happen indefaultLoad, which in the case of a YAML loader would not have been called. In order for such a user loader to not be unnecessarily duplicative of Node’s internals, we would also need to make progress on #26 (comment), making available some utility functions for some of the logic that Node does withindefaultResolveanddefaultLoad, so that this user loader could defer to existing Node logic for loading a file from disk but without needing to also do any assertion validation thatdefaultLoadwould do.So anyway, in this model nothing happens before
resolveor betweenresolveandload, or afterload. The validation is done indefaultLoad, if a user loader calls it or if no user loaders are present.I think loader authors need the assertions info in both hooks, because
resolvemight want to resolve to a different URL based on the assertion, and inloada loader author might want to do a custom validation, which they would need the assertion for (such as this YAML example). If there was some way for authors to pass arbitrary data fromresolveintoload, that would alleviate the need for us to orchestrate this.The module couldn’t be added to the cache until after
loadreturns with the module’s source, and since the validation would happen duringloadwe would never have cached modules that would have failed the validation check. This would mean, I think, that we should never have race conditions.Also @aduh95 you’re always welcome to join the loaders meetings, you don’t need an invitation. Subscribe to this repo and you should see an issue created a few days before each meeting with the meeting details.
ljharb commentedon Oct 6, 2021
The language spec actively prohibits a different module to be loaded based on the assertion - in other words,
resolveshouldn't even get the assertion information. The assertion only determines if the module will load, or will fail.aduh95 commentedon Oct 6, 2021
It could be either, considering that no one is supporting implementation nodejs/node#40250, I was thinking about re-using it.
Maybe we are talking about a different cache: if a user imports twice the same specifier, and
resolve()returns the sameurlfor both, I don't think we want to callload()twice, I'd say we want use the same module job – even thoughload()hasn't returned the final source yet.To be precise, the spec proposal says
moduleRequest.[[Assertions]] must not influence the interpretation of the module or the module specifier; instead, it may be used to determine whether the algorithm completes normally or with an abrupt completion.The question is if we can let user provided loaders violate the spec if they dare. I've added a line to the table for scenario on whichresolve()doesn't get the import assertion.targos commentedon Oct 6, 2021
What about adding a new loader hook, which would only return whether the assertion passes or not?
targos commentedon Oct 6, 2021
@GeoffreyBooth
About:
From my point of view, this example is invalid.
I think that assertions should be about what the specifier is translated to, not what it's being translated from.
If the code is run with a YAML loader that translates YAML to JSON, then the assertion in the code should be
{ type: 'json' }, and the loader doesn't need the assertion to behave correctly.aduh95 commentedon Oct 6, 2021
That's definitely worth considering. Also maybe something that would correspond to the
HostGetSupportedImportAssertionsabstract operation in the spec (I could imagineload()hook returning this list).I think the idea is that if a TC39 proposal for YAML modules comes up, someone could use a loader hook to emulate support for it in Node.js.
GeoffreyBooth commentedon Oct 6, 2021
We can’t really add more loader hooks because of chaining. We just went through a monthslong redesign to collapse four hooks to two to make them chainable. However, making utility functions that split out these smaller pieces that are within the larger hooks would be doable, and should provide many of the same benefits. So if you’re writing a loader that only wants to override the assertion validation, for example to make it always pass, you could create a
loadhook that pulls in the utility functions for ‘load this file from disk’ and ‘determine the format of this file’ and then write your own code (or lack thereof) for the validation.GeoffreyBooth commentedon Oct 6, 2021
Yeah. For example, according to https://web.dev/css-module-scripts/, Chrome already supports this:
I don’t know what a loader would convert this to; ESM JavaScript that is just
export default 'the css contents as a string'? So I thought YAML would be a better example. But the YAML example could also be converted by our loader intoexport default {"...where the export is the JSON dropped into JavaScript, and so then the loader returns{ source: jsString, format: 'module' }. In that case, if the assertion needs to match the return value from the loader then the original import would have needed to lack an assertion, because the loader is outputting JavaScript rather than JSON. This feels wrong to me, as the user shouldn’t need to change the original source code based on which custom loader they’re using.troywweber7 commentedon Jan 6, 2023
Importing YAML directly into a node.js program seems like it could be pretty useful. How far away are we from such a reality?
GeoffreyBooth commentedon Jan 6, 2023
It’s unlikely to be ever supported natively, unless spec committees add support for it like they have for JSON; but you can write a loader to add custom support for it today.
troywweber7 commentedon Jan 6, 2023
What is the best resource to get me started if I wanted to write something like that? I found this article online but I'm wondering if there is a more proper document for me to refer to when trying to write a loader?
I'm mostly curious about this for my own edification. I doubt I'd be able to write anything official, but it could be helpful internally or for a side project.
Thanks for your quick feedback and your time!
UPDATE: official documentation is probably where I should start...
GeoffreyBooth commentedon Jan 6, 2023
Yes, the official docs should be all you need. The “transpiler loader” example is the pattern you’d want to follow; you’re essentially transpiling YAML into JSON.