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

How should unrecognized attributes or types be handled? #21

Open
littledan opened this issue Nov 8, 2019 · 15 comments
Open

How should unrecognized attributes or types be handled? #21

littledan opened this issue Nov 8, 2019 · 15 comments

Comments

@littledan
Copy link
Member

One option is to ignore them. However, that risks forward compatibility issues if they are later given semantics. I would suggest aborting the whole module graph when these are found, just like when an invalid module specifier or syntax error is there.

@devsnek
Copy link
Member

devsnek commented Nov 8, 2019

failing the graph is also problematic though. i may want to specify an option that one host understands and needs but isn't understood or needed by other hosts, in the name of having maximal compatibility.

On the other hand, an attribute may also be specifying a certain security invariant and it would be bad if it loaded without that invariant being upheld.

Either way seems pretty not fun.

@littledan
Copy link
Member Author

Yeah, this is a tricky balance. I think the strictness here is orthogonal here to whether the metadata is out of line in a separate file, inline as part of the module specifier, or using this module attributes proposal. We should think more about this aspect.

@gibson042
Copy link

Let's assume a starting point in which everything is working—all attributes are understood and the module graph loads successfully. Now the author wants to add a new attribute that some hosts will understand and others will not.

If the purpose of the new attribute is to increase strictness (SRI, enforcement of purity/lack of side effects, absence of top-level await, etc.), then the enhanced hosts should start verifying it as a requirement for successfully loading the module graph, while the hosts that don't understand it should ignore it. There is a risk of typographical errors corrupting intentions such that the desired strictness is not communicated (e.g., misspelling an attribute name), but addressing that seems to fall in the domain of linting, although I would encourage hosts to surface warnings about ignored attributes.

If the purpose of the new attribute is instead to improve efficiency (e.g., leveraging caches or pre-compilation) or control characteristics of module access (HTTP request headers such as User-Agent or Accept, filesystem credentials, logging verbosity and/or exposure, etc.), then again it seems like hosts not understanding the attribute should ignore it.

So my preference would be "ignore unknown attributes". And if we encounter a case in which that fails, I would propose an attribute expressing a requirement for hosts to fail upon encountering an unknown attribute (e.g., "strictLoading": "true").

@littledan
Copy link
Member Author

@gibson042 's argument makes sense to me (though I don't understand the suggestion in the last paragraph). Still, once an attribute is defined, then we may want to be strict about it, right? E.g., an unknown type: would be a parsing/linking error.

@devsnek
Copy link
Member

devsnek commented Nov 9, 2019

probably depends on the attribute but for type it would make sense to fail linking

@ljharb
Copy link
Member

ljharb commented Nov 9, 2019

Wouldn’t an unknown type still be something you’d want to be ignored, just like an unknown attribute? (for the same reasons)

@devsnek
Copy link
Member

devsnek commented Nov 9, 2019

it's unknown attribute name vs unknown attribute value. the validation of an attribute value would depend on its meaning, and in the case of type it means you're trying to control the type that something is. if the host doesn't know that type, how can it be expected to load it correctly? failing seems reasonable.

@ljharb
Copy link
Member

ljharb commented Nov 9, 2019

The type here isn’t granting the ability to load it, it’s constraining it’s existing ability to do so. An unknown type would simply mean there’s no constraint.

@gibson042
Copy link

type is doing more than constraining loading, it's defining how to load. It would be wrong to try interpreting CSS/HTML/etc. as ECMAScript module source in the presence of a clear but not understood author signal. I can imagine an attribute to opt-in to lenience for fallback behavior (e.g., preferring Wasm but still supporting pure ES hosts), but IMO default behavior for type should be to fail upon encountering an unknown value.

@ljharb
Copy link
Member

ljharb commented Nov 9, 2019

This is a very different proposal if it's mandating that non-JS imports be annotated with a type. Is that what it's trying to do? I read it as allowing it, and then leaving it to implementations (like web browsers) to mandate it.

@littledan
Copy link
Member Author

Layering-wise, I see the host as technically having the ability to permit other types where no type was specified. I hope we can find a common set of semantics that is useful across environments, but I doubt that would be formally mandated. Sounds like we should probably have some more issues to go in depth on figuring out Web and Node semantics separately, so we can be more concrete here (since I imagine a lot of us care about those two).

@bahrus
Copy link

bahrus commented Nov 9, 2019

Maybe provide a prefix for custom attributes, that may aid build processes, like embedding the resource in some cases?

@littledan
Copy link
Member Author

littledan commented Nov 9, 2019

I'm not sure if prefixes make sense, since it's likely that something that starts out with meaning in just one environment may eventually be given an analogous meaning in other environments, even if that decision isn't made upfront.

@Jack-Works
Copy link
Member

Maybe...

import 'path' with { "data-my-meta": "x", type: "html", noExecute: true }
// data-my-meta is ignorable

Or

import 'path' as { '!type': 'json', 'mutable': false }
// !type is not ignorable

Or

import 'path' as { type: 'json', _: { my: true } }
// data in the _ is ignoreable

@littledan
Copy link
Member Author

I want to propose that hosts determine how unrecognized attributes or types are handled. Prior to Stage 3, I want to have the integration for at least some hosts worked out (e.g., an HTML PR that has some level of agreement in that space). Would it be OK to go to Stage 2 leaving this question as considered something for hosts to decide, or does Stage 2 require that we determine that the JavaScript specification would make a requirement on hosts here?

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

6 participants