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

Add more default extensions to resolve .mjs & .wasm & .json #8327

Merged
merged 1 commit into from
Sep 19, 2018

Conversation

KyleAMathews
Copy link
Contributor

fixes #8246
fixes #7581

Copy link
Contributor

@kkemple kkemple left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@KyleAMathews
Copy link
Contributor Author

These are all default resolvable extensions in webpack so we should add them. NPM packages are starting to ship .mjs files which people are running into trouble with.

Copy link
Contributor

@DSchau DSchau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, apart from my comment.

packages/gatsby/src/bootstrap/index.js Show resolved Hide resolved
@jquense
Copy link
Contributor

jquense commented Sep 19, 2018

my only concern here is it dramatically increases the pathnames webpack has to stat to resolve one file...my hunch would be to only support what Node does out of the box

@KyleAMathews
Copy link
Contributor Author

my hunch would be to only support what Node does out of the box

Agreed — other than wasm (and jsx), this is now what node support out of the box.

@KyleAMathews
Copy link
Contributor Author

Yarn registry killed appveyor :-| merging as otherwise is working.

@KyleAMathews KyleAMathews merged commit 60e1361 into master Sep 19, 2018
@KyleAMathews KyleAMathews deleted the add-resolvable branch September 19, 2018 19:26
@alpgumus
Copy link

This doesn't seem to fix #7581 unless:
(actually, it is complaining about mjs files unless:)

actions.setWebpackConfig({
    module: {
      rules: [
        {
          test: /\.mjs$/,
          type: 'javascript/auto',
        },
      ],
    },
})

@KyleAMathews
Copy link
Contributor Author

Hmm ok. Could you open a new issue with a reproduction?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants