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

<script> type attribute and WorkerOptions dict type are a little overloaded now #17

Closed
dandclark opened this issue Nov 8, 2019 · 11 comments
Milestone

Comments

@dandclark
Copy link
Collaborator

<script>'s type attribute is a little odd with this change.
<script type="module"> means a JavaScript module, while <script type="webassembly"> also means a module, but a different type of module. This is not super intuitive, but I don't really see a backwards-compatible workaround. I guess this can be justified by saying that there is no such thing as a 'classic' WebAssembly script (is that right?) so the distinction is clear in context.

Same thing with Workers; https://github.com/littledan/proposal-module-attributes#worker-instantiation conflicts a bit with the current way that the worker options dict is used to decide whether a Worker should be classic or module: https://html.spec.whatwg.org/multipage/workers.html#worker-processing-model

const worker = new Worker('resource.json', {type:'module'});

or

const worker = new Worker('resource.json', {type:'classic'});

Now we would also have

const worker = new Worker('resource.json', {type:'webassembly'});

Which is a little confusing because it is also a module, just not a JS one.

I don't see a good way to address this as changing <script type="module"> is obviously a non-starter. I thought I'd make a note of the issue though, as this is likely a criticism that we'll have to speak to.

@littledan
Copy link
Member

Yes, I was thinking of using a secondary label, but I thought the overload was a good trade-off since those don't have many possible values anyway. Note that #7 points to more possible overloading.

@bmeck
Copy link
Member

bmeck commented Nov 8, 2019

@littledan the web has few values, but that is not necessarily true of other environments. Deno has typescript, node loaders allow arbitrary formats, etc.

@littledan
Copy link
Member

@bmeck Maybe you meant to post that on #8 or #10?

@bmeck
Copy link
Member

bmeck commented Nov 8, 2019

@littledan it was in direct reply to:

since those don't have many possible values anyway

This isn't necessarily true in other environments especially with Worker APIs.

@littledan
Copy link
Member

Cross-references would be useful here.

@bmeck
Copy link
Member

bmeck commented Nov 8, 2019

@littledan
Copy link
Member

Reviewing these, I'm having trouble finding any currently shipping feature that would lead to compatibility/interoperability issues with using a { type: ... } option as a parameter to initialize workers. Am I missing something?

@bmeck
Copy link
Member

bmeck commented Mar 16, 2020

@littledan currently workers and node's main entrypoint (idk who does that currently) can load WASM via using type:module. I'm stating that the naming collision here doesn't make sense to try and overload.

@littledan
Copy link
Member

Right, so, I was suggesting that we could join a namespace that has just a few entries, and work through the collisions. (And I imagine that, if you're talking about Node, you mean an experimental flag.) It still seems to me like that's a possible path.

But I'm also open to passing these options in a separate place. For example, the API for workers on the Web could be:

const worker = new Worker('resource.json', {with: {type:'webassembly'}});

@littledan littledan added this to the stage 3 milestone May 20, 2020
@littledan
Copy link
Member

I'm leaning towards using the syntax described above:

const worker = new Worker('resource.json', {with: {type:'webassembly'}});

This avoids all risk of namespace collisions. It's also used in import().

Also, for HTML tags, the withtype attribute would be used:

<script src="foo.wasm" type="module" withtype="webassembly"></script>

This is all checked into master.

I think these host integration things are something that we should work out by Stage 3.

@littledan
Copy link
Member

I'm closing this issue, as the overloading has been removed.

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

No branches or pull requests

3 participants