Nuance the handling of imports in ESM-integration #37
Conversation
|
|
||
| Lastly, considering the new exportable types, a module export would naturally | ||
| produce a `WebAssembly.Module` object. For an instance export, the JavaScript | ||
| correspondence already established by [ESM-integration] is a [Namespace Object]. |
There was a problem hiding this comment.
Interesting point, this implies the possibility of namespace construction for all WebAssembly.Instance objects right? This might require an ECMA262 spec change / WebAssembly JS API change to reflect all WebAssembly.Instance objects as having a Namespace Exotic Object representation that can be used / constructed.
There was a problem hiding this comment.
That's a good point. Technically, it would be the "exports object" (i.e., WebAssembly.Instance.exports) that we might want to reframe as a Namespace Object. IIRC, I think this was something we explicitly considered when defining the JS API in the first place and it's the reason for the final freezing step of creating the exports object (so that we could change it to a Namespace Object in the future without breaking existing code that depended on mutating the POJO). The reason for not doing this sooner was, again IIRC, that Namespace Objects weren't implemented at the point in time we were shipping the JS API.
There was a problem hiding this comment.
Yeah I think it would be important to unify the interface now to ensure consistency with these representations. Was the plan to transparently upgrade "exports" or introduce a new property? Since the exports object is frozen it seems that should be possible, but potential edge cases include the prototype difference of the Namespace Exotic object (eg no exports.hasOwnProperty() so users doing that wouldn't work if exports changes to a namespace). It might still be safer to construct a new Instance.namespace in the JS API. Would it be worth getting an issue going for that?
There was a problem hiding this comment.
My preference would be to try to upgrade exports in-place and see if anything breaks (say, by having a browser ship the upgrade in a pre-release channel for a while as an experiment). Filing an issue makes sense but, to be clear, I think this would be a general JS API change, independent of Module Linking. Since it's ESM-related, maybe the right repo is ESM-integration?
|
@guybedford Thanks so much for the careful review and thoughtful questions! |
This PR attempts to incorporate Guy's ideas in #32. Already the line suggested to be removed was removed by #35. This PR adds the idea suggested in the final comments saying that single-level imports start out with Module Records, and then perform the default-value conversion when needed, thereby allowing adapter modules to directly import Namespace Objects if they want.
I also realized that, in terms of case analysis, the "module import" case goes first because it doesn't start with a Module Record (which iiuc is the "instance" of the ESM world); it asks the ESM loader for a totally different (new) thing.