tc39 / proposal-import-attributes

Proposal for syntax to import ES modules with assertions
https://tc39.es/proposal-import-attributes/
Apache License 2.0
599 stars 26 forks source link

How should unrecognized attributes or types be handled? #21

Open littledan opened 5 years ago

littledan commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

@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 commented 5 years ago

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

ljharb commented 5 years ago

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

devsnek commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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

littledan commented 5 years ago

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 commented 4 years ago

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 commented 4 years ago

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?