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

Type of builtin modules #35

Open bmeck opened 4 years ago

bmeck commented 4 years ago

It seems builtin modules would also want a type associated with them? IDK if this has been thought about but they would be ensured to not go through user-land (provide by language or host?).

import 'builtin:thing' with type:'???';
xtuc commented 4 years ago

I would expect the same semantics to apply for consistency. Probably that would be useful for tooling still.

bmeck commented 4 years ago

@xtuc I don't understand that comment, which specific semantics? requiring a type be set? and if so, to what?

ljharb commented 4 years ago

With every other module, there’d be only one explicit type value that resulted in successfully loading the module. What single type value would i need to choose to ensure that a builtin module loads?

bmeck commented 4 years ago

Part of my worry here is that if you do set the type it means that polyfilling needs to be taken into account like all the other cases of polyfilling.

xtuc commented 4 years ago

I suppose that for a JSON JavaScript builtin module the type: "json" would be required, if we require it for user modules.

From a developer perceptive, I'm not sure how the host will signal that a buitlin module is a JSON module.

bmeck commented 4 years ago

@xtuc for non-JSON/non-side effecting builtin modules would that mean they are treated like normal JS with side effects and likely wouldn't have a required type? That seems it would potentially break the idea of specifying the type guarding against effects, it seems builtins would need a separate value from JS as loading host builtins doesn't have the same implications of loading potentially having arbitrary effects.

justinfagnani commented 4 years ago

I suppose that for a JSON JavaScript builtin module the type: "json" would be required, if we require it for user modules.

This proposal isn't about requiring metadata, but allowing for it. I don't know why Node would require it for built-in modules.

ljharb commented 4 years ago

I read this issue as asking what type value is allowed for builtin modules - presumably if it’s “json” then only a json-serializable default export would exist.

MylesBorins commented 4 years ago

my assumption is that builtins that are JavaScript modules would not require a type.

On Wed, Dec 18, 2019 at 11:45 AM Jordan Harband notifications@github.com wrote:

I read this issue as asking what type value is allowed for builtin modules

  • presumably if it’s “json” then only a json-serializable default export would exist.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/tc39/proposal-module-attributes/issues/35?email_source=notifications&email_token=AADZYV3BY3IEKK6UOR2RYFLQZJHT3A5CNFSM4J3V4GMKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHGXZ7Y#issuecomment-567115007, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADZYV55IV3YJIGZ4SPPVF3QZJHT3ANCNFSM4J3V4GMA .

bmeck commented 4 years ago

@justinfagnani

This proposal isn't about requiring metadata, but allowing for it. I don't know why Node would require it for built-in modules.

This is not about Node specifics at all; JS itself is looking at builtin language modules. Those are much more pertinent.

@MylesBorins

my assumption is that builtins that are JavaScript modules would not require a type.

I assume they must have a type for the same security concerns as JSON modules are being touted as needing to guard against accidental effects on import. If builtins do not require a type, it questions why JSON needs a type and opens a collision where polyfilling can introduce side effects since the import sites would match arbitrary JS. E.G.

// ensures no side effects when loading, same as the argument for JSON
// unclear on how to polyfill
import 'std:stdthing' with type="builtin";

vs.

// allows effects if polyfilled
import 'std:stdthing';
MylesBorins commented 4 years ago

My understanding of the security concern was not simply side effects but rather that a MITM or nefarious server could send a malicious payload. In the case of a built-in nothing is being sent over the network and this seems unrelated to the original security concern.

On Wed, Dec 18, 2019 at 2:38 PM Bradley Farias notifications@github.com wrote:

@justinfagnani https://github.com/justinfagnani

This proposal isn't about requiring metadata, but allowing for it. I don't know why Node would require it for built-in modules.

This is not about Node specifics at all; JS itself is looking at builtin language modules. Those are much more pertinent.

@MylesBorins https://github.com/MylesBorins

my assumption is that builtins that are JavaScript modules would not require a type.

I assume they must have a type for the same security concerns as JSON modules are being touted as needing to guard against accidental effects on import. If builtins do not require a type, it questions why JSON needs a type and opens a collision where polyfilling can introduce side effects since the import sites would match arbitrary JS. E.G.

// ensures no side effects when loading, same as the argument for JSON// unclear on how to polyfillimport 'std:stdthing' with type="builtin";

vs.

// allows effects if polyfilledimport 'std:stdthing';

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/tc39/proposal-module-attributes/issues/35?email_source=notifications&email_token=AADZYVYDN6DJSEX5VCNLTILQZJ34VA5CNFSM4J3V4GMKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHHHXWY#issuecomment-567180251, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADZYV4FY2SKDUNWX6O2OZLQZJ34VANCNFSM4J3V4GMA .

bmeck commented 4 years ago

@MylesBorins with import maps it could be redirected, I am unclear on the difference if that is the case.

justinfagnani commented 4 years ago

I'm really unclear on why the import metadata proposal would have anything to say on built-in modules. Wouldn't the host providing built-in modules decide what to do with the metadata?

bmeck commented 4 years ago

@justinfagnani it depends, but this is a cross environment interoperability concern and to date the main (and sometimes framed as sole) intent of this proposal is about supporting a security model for the web. This is pertinent for JS builtin modules since if the interoperability of builtins across hosts seems to be affected by this proposal and the intents of how it will be used.

justinfagnani commented 4 years ago

@bmeck have we heard anyone propose blocking built-in modules on security grounds?

bmeck commented 4 years ago

@justinfagnani not to my knowledge, but that is a separate issue if you find one.

MylesBorins commented 4 years ago

I think the combination of built ins + import maps is out of scope for this. Although the interaction between important maps and this proposal does seem important... especially if an import maps is redirecting one type of module to another

On Wed, Dec 18, 2019 at 2:59 PM Bradley Farias notifications@github.com wrote:

@justinfagnani https://github.com/justinfagnani not to my knowledge, but that is a separate issue if you find one.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/tc39/proposal-module-attributes/issues/35?email_source=notifications&email_token=AADZYV5DD6U56NQLKTYEFXLQZJ6KZA5CNFSM4J3V4GMKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHHJ4DA#issuecomment-567189004, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADZYVY5TCHNH2MOFD3QQ5LQZJ6KZANCNFSM4J3V4GMA .

bmeck commented 4 years ago

@MylesBorins this proposal affects sharing of JS code between environments. If we want to be more abstract about interoperability concerns that seems like it is possible, but we would need to remove lots of the talking points about type= for me to be comfortable with that.

littledan commented 4 years ago

It's hard for me to see why built-in modules would need a type rather than acting as JS modules, though if someone working on built-in modules has a reason for this, it'd be good to hear. cc @mattijs @msaboff

littledan commented 4 years ago

I'd still suggest that, in general, there's no particular need for built-in modules to have a type declared--they can go typeless, like JS. Polyfilling built-in modules is a bigger topic that depends on parts of the proposal that aren't decided yet, so I'm not really sure how to even think about it here. If anyone has a reason why built-in modules would need an explicit type:, it'd be good to hear it. However, this would be a decision for the built-in modules proposal champions to make. So, I'm leaning towards closing this issue with no action taken.