Skip to content
This repository has been archived by the owner on Apr 16, 2020. It is now read-only.

Allow future extensions without package.json#type field #38

Closed
jkrems opened this issue Feb 16, 2019 · 10 comments
Closed

Allow future extensions without package.json#type field #38

jkrems opened this issue Feb 16, 2019 · 10 comments

Comments

@jkrems
Copy link
Contributor

jkrems commented Feb 16, 2019

In a package without additional boilerplate (e.g. "type" field), we currently default to treating unknown extensions as CommonJS. This means that we won't be able to support any future extensions (.wasm, .html, ...) unless they are handled by CommonJS.

@devsnek
Copy link
Member

devsnek commented Feb 16, 2019

This means that we won't be able to support any future extensions unless they are handled by CommonJS

can you expand on why? what's stopping us from just adding more supported formats to the esm loader?

@jkrems
Copy link
Contributor Author

jkrems commented Feb 16, 2019

It would be a breaking change because they have been forwarded to the CommonJS pipeline before. If somebody has their own custom HTML require._extensions entry, that would get .html imports and it would suddenly stop working.

@GeoffreyBooth
Copy link
Member

Do you mind explaining where this is happening? As far as I understand the implementation, import statements currently only support .js and .mjs and .cjs; I remember us discussing having them throw on .json and .node and unknown extensions. The discussion was around WASM, so that when someday an import of .wasm is allowed it’s not a breaking change (which it would be if import './foo.wasm' was currently a CommonJS interop thing where foo.wasm is treated as CommonJS like foo.cjs would be).

@ljharb
Copy link
Member

ljharb commented Feb 16, 2019

Unknown extensions should fail outright, just like they do now - nothing should be "forwarded" to the CommonJS pipeline without an explicit indicator that it's CJS.

@jkrems
Copy link
Contributor Author

jkrems commented Feb 16, 2019

Do you mind explaining where this is happening?

import "some-dep/foo.zapp";

Afaict this make us load a file with extension .zapp as CJS because that's what ESM_FORMAT returns in the else branch. Without additional boilerplate in some-dep/package.json it's impossible to prevent this / preserve our ability to handle .zapp differently in the future.

@ljharb
Copy link
Member

ljharb commented Feb 16, 2019

Seems like "if/else" isn't the logic we wan't; if it's unknown imo it should throw.

@GeoffreyBooth
Copy link
Member

import "some-dep/foo.zapp" should throw, at least according to our discussions and I think also according to the proposal. If it doesn’t, that’s a bug in the implementation.

@jkrems
Copy link
Contributor Author

jkrems commented Feb 16, 2019

If type isn't set, it will go to the last if/else in ESM_FORMAT and that's just a plain else. No check on supported extensions.

@devsnek
Copy link
Member

devsnek commented Feb 16, 2019

i'm fairly sure the eventual plan is to have the main loader be the esm loader, which would call into cjs for cjs, and handle everything else itself.

@Trott
Copy link
Member

Trott commented Apr 9, 2020

Is this an issue that can be closed at this time? Or transfered to the main repository?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants