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

Type of builtin modules #35

Open
bmeck opened this issue Dec 17, 2019 · 20 comments
Open

Type of builtin modules #35

bmeck opened this issue Dec 17, 2019 · 20 comments

Comments

@bmeck
Copy link
Member

bmeck commented Dec 17, 2019

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
Copy link
Member

xtuc commented Dec 18, 2019

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

@bmeck
Copy link
Member Author

bmeck commented Dec 18, 2019

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

@ljharb
Copy link
Member

ljharb commented Dec 18, 2019

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
Copy link
Member Author

bmeck commented Dec 18, 2019

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
Copy link
Member

xtuc commented Dec 18, 2019

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
Copy link
Member Author

bmeck commented Dec 18, 2019

@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
Copy link

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
Copy link
Member

ljharb commented Dec 18, 2019

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
Copy link
Member

MylesBorins commented Dec 18, 2019 via email

@bmeck
Copy link
Member Author

bmeck commented Dec 18, 2019

@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
Copy link
Member

MylesBorins commented Dec 18, 2019 via email

@bmeck
Copy link
Member Author

bmeck commented Dec 18, 2019

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

@justinfagnani
Copy link

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
Copy link
Member Author

bmeck commented Dec 18, 2019

@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
Copy link

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

@bmeck
Copy link
Member Author

bmeck commented Dec 18, 2019

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

@MylesBorins
Copy link
Member

MylesBorins commented Dec 18, 2019 via email

@bmeck
Copy link
Member Author

bmeck commented Dec 18, 2019

@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
Copy link
Member

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
Copy link
Member

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.

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