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

fix(config): Add .mjs as an extension for Webpack to resolve #204

Merged
merged 2 commits into from
Jan 20, 2021

Conversation

nwalters512
Copy link
Contributor

#107 dropped .mjs from the list of extensions Webpack will resolve. This is causing problems for us - graphql is in the import path of some of our components, and graphql/graphql-js#1272 is biting us. This cannot be fixed in userspace via webapckConfig in playroom.config.js, since the .mjs extension needs to appear before the .js extension, but the user-provided resolve.extensions is concatenated to the end of Playroom's base Webpack config.

An alternative solution would be to customize the way that webpack-merge merges resolve.extensions so that user-provided extensions always appear first. I'd be happy to pivot this PR to that if that's what the maintainers would prefer.

@nwalters512 nwalters512 requested a review from a team as a code owner November 18, 2020 20:30
@maraisr
Copy link
Contributor

maraisr commented Nov 19, 2020

mjs actually require a bit more than just resolution. You'd need to couple this with a webpack module rule:

config.module.rules.push({
	test: /\.mjs$/,
	include: /node_modules/,
	type: 'javascript/auto',
});

both of this you could just do in consume land with the webpack override.

@nwalters512
Copy link
Contributor Author

Ah, interesting! Thanks for the tip. Before making this PR, I had tested locally by editing Playroom files in node_modules, and the build succeeded just with the addition of the .mjs extension. When I tested with Webpack config overrides, I didn't add the module rule. After adding that, things seem to work. Here's a simplified version of what my Playroom config looks like now:

// playroom.config.js
module.exports = {
  webpackConfig: () => ({
    resolve: {
      extensions: ['.mjs'],
    },
    module: {
      rules: [
        {
          test: /\.mjs$/,
          include: /node_modules/,
          type: 'javascript/auto',
        },
      ],
    },
  }),
};

Would be happy to add the module rule to this PR so that things "just work" for other folks.

@maraisr
Copy link
Contributor

maraisr commented Nov 19, 2020

It all stems from Webpack not handling mjs properly. Or properly, who knows. The extension is fractured and nobody really knows! But this plugin resolved all:
https://github.com/lukeed/webpack-modules

@michaeltaranto michaeltaranto merged commit 17c46fa into seek-oss:master Jan 20, 2021
@nwalters512 nwalters512 deleted the fix/mjs-extension branch January 21, 2021 02:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants