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

Runtime error when loading mjs files #8655

Closed
corydeppen opened this issue Sep 30, 2018 · 5 comments
Closed

Runtime error when loading mjs files #8655

corydeppen opened this issue Sep 30, 2018 · 5 comments
Labels
good first issue Issue that doesn't require previous experience with Gatsby status: confirmed Issue with steps to reproduce the bug that’s been verified by at least one reviewer. type: bug An issue or pull request relating to a bug in Gatsby

Comments

@corydeppen
Copy link

Description

I've read through several recent discussions about supporting mjs files and I see #8327 was merged to include the extension, however I continue to see the original require is not defined runtime error after adding the change to include the extension. From what I've seen in my app and in several repos (e.g. aws-amplify/amplify-js#686, facebook/create-react-app#5151), the test for mjs still needs to be added to the webpack JS loader rules.

Steps to reproduce

My app is based on the using-multiple-providers example, along with the gatsby-source-graphql plugin.

The following custom webpack config allows the page to load as expected:

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

Expected result

Should not see a runtime error when using packages that include mjs files.

Actual result

Uncaught ReferenceError: require is not defined
    at eval (Users/{user}/{path-to-app}/node_modules/graphql/jsutils/instanceOf.mjs:34)
    at Module../node_modules/graphql/jsutils/instanceOf.mjs (commons.js:3856)
    at __webpack_require__ (commons.js:725)
    at fn (commons.js:102)
    at eval (Users/{user}/{path-to-app}/node_modules/graphql/type/definition.mjs:46)
    at Module../node_modules/graphql/type/definition.mjs (commons.js:4180)
    at __webpack_require__ (commons.js:725)
    at fn (commons.js:102)
    at eval (Users/{user}/{path-to-app}/node_modules/graphql/type/validate.mjs:4)
    at Module../node_modules/graphql/type/validate.mjs (commons.js:4252)

Environment

  System:
    OS: macOS High Sierra 10.13.6
    CPU: x64 Intel(R) Core(TM) i7-8850H CPU @ 2.60GHz
    Shell: 5.5.1 - /usr/local/bin/zsh
  Binaries:
    Node: 8.11.3 - /usr/local/bin/node
    Yarn: 1.9.4 - /usr/local/bin/yarn
    npm: 6.1.0 - /usr/local/bin/npm
  Browsers:
    Chrome: 69.0.3497.100
    Safari: 12.0
  npmPackages:
    gatsby: ^2.0.0 => 2.0.7 
    gatsby-plugin-emotion: ^2.0.5 => 2.0.5 
    gatsby-plugin-react-helmet: ^3.0.0 => 3.0.0 
    gatsby-source-filesystem: ^2.0.1 => 2.0.1 
    gatsby-source-graphql: ^2.0.2 => 2.0.2 
    gatsby-transformer-remark: ^2.1.3 => 2.1.3 
@KyleAMathews
Copy link
Contributor

This sounds like a reasonable change to add to core. Can you create a reproduction site that currently doesn't work?

@corydeppen
Copy link
Author

@kakadiadarpan kakadiadarpan added status: confirmed Issue with steps to reproduce the bug that’s been verified by at least one reviewer. type: bug An issue or pull request relating to a bug in Gatsby status: inkteam to review labels Oct 1, 2018
@DSchau DSchau added Hacktoberfest good first issue Issue that doesn't require previous experience with Gatsby and removed status: inkteam to review labels Oct 2, 2018
@DSchau
Copy link
Contributor

DSchau commented Oct 2, 2018

@corydeppen thanks for the repro and more detail on the fix! This is a great issue that someone in the community (or yourself if you're interested 😄) could tackle.

For other coming here, this will involve the following changes:

Updating our webpack.config.js and rules in webpack-utils.js to default to adding the .mjs rule

@akadop
Copy link
Contributor

akadop commented Oct 2, 2018

I 100% agree with this pr.

ran into a similar issue at work about a week ago. we use NextJS for a lot of the live projects. next recently released version 7 -- and it now uses babel 7 and webpack 4 under the hood.

First thing noticed when upgrading to Next7:

  • Any repo using graphql@0.13.2 (which is standard for any project using react-apollo, apollo-client) would result in a failure to initialize react. Webpack could not/would not automatically pick up the .mjs extension.

    error displayed in browser: Error was not caught ReferenceError: "require is not defined"

  • these issues resolved when including the fixes laid out in this pr (different codebase, sure, but same context):

        {
            test: /\.mjs$/,
            include: /node_modules/,
            type: 'javascript/auto'
          }
 config.resolve.extensions = ['.mjs', '.tsx', '.ts', '.js', '.jsx', '.json']

Here is a ticket opened up in the next codebase, along with the same solution.
vercel/next.js#5233 (comment)

@akadop
Copy link
Contributor

akadop commented Oct 2, 2018

#8717

DSchau pushed a commit that referenced this issue Oct 2, 2018
deals with issues:
 - Fixes #8655
 - other various `.mjs` issues after the switch to webpack 4

Add webpack rule to explicitly state that `.mjs` files are to be loaded in with the default webpack 4 js loader. Comments also left for future references.

@KyleAMathews 's pr (https://github.com/gatsbyjs/gatsby/pull/8327/files) was correct to add the extensions -- but `.mjs` needs to be before the `.js` extension. Looks like someone changed it in a PR afterwards. In any case, this is half of the solution. We need to explicitly state what loader webpack should use when resolving these files.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Issue that doesn't require previous experience with Gatsby status: confirmed Issue with steps to reproduce the bug that’s been verified by at least one reviewer. type: bug An issue or pull request relating to a bug in Gatsby
Projects
None yet
Development

No branches or pull requests

5 participants