-
Notifications
You must be signed in to change notification settings - Fork 26
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
Semantics of this feature in Node.js #25
Comments
node has no need to use this type information at all; it would have to be optional for everything. |
@ljharb Are you saying you agree with Option B, or do you have concerns with it? Under Option B, the |
Option B says optional for non-JS types; in node it’d be optional for every type. |
Heh, the idea is that it's optional for non-JS types because of course it's just not provided for JS types... so yes, it'd be optional for all types. It sounds like you're speaking on behalf of Node about what its semantics would be. Has the Node Modules team made a decision about this yet? |
I've edited the original post to make the clarification. |
To clarify, I’m not speaking on behalf of node or the modules group; however, i can’t imagine that the UX tax of providing a type inline would gain consensus in either venue. |
Is Option B acceptable to you, in your personal opinion about developer UX (modulo the question of how unknown types are processed)? |
The concerns motivating this proposal don’t exist in node, so if the syntax exists in the language, option B seems like the only non-disruptive way it could work (since compatible web code still needs to be able to work in node) |
I'd leave this to the node and the modules WG rather than attempting to state only a few possible options. |
Yes, I definitely didn't mean that these are the only two options, and am supportive of this being decided by the Node Modules Team. I wanted to start the discussion, though. You can see that I've also filed issues about the web semantics, even though those would also happen in web standards bodies. |
@littledan I've added this to the modules WG. I'm just not comfortable speaking on behalf of the WG as neither of those options seem likely in personal viewpoint. |
To add to the node use case: There have been discussions around supporting |
Of note, some people might be skeptical of the need for mandating the |
I would count myself in that camp. But to me that applies to the browser as well: In many (most?) cases, I would expect the JSON being imported to be from the same domain etc., controlled by the app owner. So forcing verbose syntax on them (instead of making it an opt-in assertion), feels punitive with little actual security win. Which is always likely to motivate people to find workarounds, undermining the few security wins left. |
@jkrems If you think that a JSON file on your web server will only be served with a Serving your website over some CDN is already a moving piece that might be misconfigured or malicious. It's more common that you might think. content-type-vs-file-extension can give you an idea. |
I'd like us to stay realistic though. If you make the argument "but what if your CDN is compromised", then that's entirely out of scope for this issue. Because it's unlikely that somebody has a CDN just for their JSON files. So why would the attacker bother to intercept the JSON load? And if your CDN is misconfigured, with or without a type annotation your website will be broken in fairly obvious ways. I'm aware of bad content-type headers. The question is: How likely is it that having this annotation will make a difference in the face of typical bad content-type scenarios. I have a hard time seeing a practical difference here apart from the safety assertion about arbitrary code execution. |
I'd also prefer a noexecute attribute instead of arbitrary metadata, but I think people are unlikely to go for that. |
@jkrems I agree that my example was not great. |
Is the intended semantics that |
@ByteEater-pl it wouldn't affect how the module is interpreted. It would only assert it is the expected type. |
Note: The semantics of this proposal in Node.js would be determined by Node collaborators and the Node Modules Team, not in this repository. This issue is purely for requirements-gathering/brainstorming.
A lot of the discussion in the issues seems to relate to how this feature would integrate into non-Web environments. For concreteness, I'm going to talk about how this might integrate into Node.js, since I know slightly more about it than other non-Web environments (but I am no Node expert). I see two general paths for the interpretation. (Note, these are not the only possible paths, they're just two that I thought of to initiate discussion)
Option A: Maximizing for similarity with the web
If we want to be maximally compatible with the web -- that is, code written for Node.js would work on the Web without a build step to handle this aspect of semantics -- then we could simply use the logic in #24, but with s/MIME type/file extension (and possibly more information in
package.json
)/. The file extension used here would be after some resolution is done, as described in #4, so it may not be what's textually included in source. This would mean that importing JSON modules would requiretype: "json"
to be textually in JS source that's used in Node.js, which differs from traditional ES6 module -> CJS transforms (which of course didn't have this syntax, but JSON modules were present anyway).Option B: Maximizing for terseness/non-redundancy
On the other hand, we could make
type:
optional fornon-JSall module types in Node.js. The semantics would be based on the template of Option A, but with the following change: if notype:
is provided, then there is no check done, and processing is based just on the file extension (and possibly more information inpackage.json
). This would mean that a build step is expected to add thewith type: "json"
or whatever else when building for the web.How this would interact with other alternatives for where to put the attributes
If the attributes are put in an out-of-band file, or in the module specifier, or in a single string, Options A and B are still both available. They make different tradeoffs about web compatibility vs lack of redundancy. This proposal would permit Node.js and other environments to make this decision themselves, regardless of the choice of format for where to put the metadata.
The text was updated successfully, but these errors were encountered: