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

Remove default ".json" and ".node" resolution for ESM #3

Closed
wants to merge 4 commits into from

Conversation

guybedford
Copy link
Contributor

@guybedford guybedford commented Aug 28, 2018

In line with simplifying towards browser compatibility, this removes the default resolution of ".json" files from ESM (although ".json" can still be resolved from CommonJS of course).

Custom loaders could still add this back as the "json" translator is still defined though here.

Looking forward to the browser compatibility discussion :)

edit (@MylesBorins): I've also added the removal of .node to this PR.

mcollina

This comment was marked as off-topic.

@devsnek
Copy link
Member

devsnek commented Aug 28, 2018

Custom loaders could still add this back as the "json" translator is still defined though here.

custom loaders should do this themselves. the translator is just dead code now.

@guybedford
Copy link
Contributor Author

Agreed they can do it through instantiate, ok I've added another commit to remove the translator entirely as well.

ljharb

This comment was marked as off-topic.

@Janpot
Copy link

Janpot commented Aug 29, 2018

@ljharb Isn't the point of this fork to just create a "minimal kernel" as a starting point, so that further features can be layered on through PRs and so there is actually something tangible to discuss about those? Removing features in this context doesn't necessarily mean that they are not useful or have no place in the eventual implementation.
And a certain interpretation (not to say that this must be it) of "minimal kernel" could be "on par with the browser"

@ljharb
Copy link
Member

ljharb commented Aug 29, 2018

@Janpot yes, but that requires consensus on what "minimal" means, and I don't think a minimal kernel is one that lacks the basic and core feature of resolution that already exists in node.

@MylesBorins MylesBorins changed the title Remove default ".json" resolution for ESM Remove default ".json" and ".node" resolution for ESM Sep 12, 2018
@MylesBorins
Copy link
Contributor

I've added the removal of .node to the PR to align with the currently proposed minimal kernel. removing cjs support will come in a future push once createRequire function has landed upstream

@MylesBorins
Copy link
Contributor

Closing in lieu of #6

@MylesBorins MylesBorins closed this Oct 2, 2018
@MylesBorins MylesBorins mentioned this pull request Oct 2, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants