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(gatsby): add mjs config to webpack and resolve correctly #8717

Merged
merged 3 commits into from
Oct 2, 2018
Merged

fix(gatsby): add mjs config to webpack and resolve correctly #8717

merged 3 commits into from
Oct 2, 2018

Conversation

akadop
Copy link
Contributor

@akadop akadop commented Oct 2, 2018

deals with issues:

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.

@akadop akadop requested review from KyleAMathews and DSchau October 2, 2018 18:25
@akadop akadop requested a review from a team as a code owner October 2, 2018 18:25
@DSchau
Copy link
Contributor

DSchau commented Oct 2, 2018

@akadop

In any case, this is half of the solution. We need to explicitly state what loader webpack should use when resolving these files.

Didn't you do that here with the javascript/auto and .mjs targeting?

@akadop
Copy link
Contributor Author

akadop commented Oct 2, 2018

@akadop

In any case, this is half of the solution. We need to explicitly state what loader webpack should use when resolving these files.

Didn't you do that here with the javascript/auto and .mjs targeting?

yeah, i meant that the previous prs was half the solution

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.

Alright! This looks good now :) I added a few tests and validated with the repro in the issue, as well as tracked regressions with a default starter.

Let's wait for the tests to pass, and then we can get this merged in!

@DSchau DSchau changed the title Add webpack rule to explicitly state that .mjs files are to be load… fix(gatsby): add mjs config to webpack and resolve correctly Oct 2, 2018
@DSchau DSchau merged commit a0cbbcb into gatsbyjs:master Oct 2, 2018
@DSchau
Copy link
Contributor

DSchau commented Oct 2, 2018

Successfully published:
 - [email protected]

lipis added a commit to lipis/gatsby that referenced this pull request Oct 3, 2018
* 'master' of github.com:gatsbyjs/gatsby: (72 commits)
  fix: fix wording in plopfile (gatsbyjs#8735)
  fix: navigateTo deprecation message (gatsbyjs#8745)
  Add deploying-to-heroku.md (gatsbyjs#8721)
  fix(docs): Link to instructions on adding to plugin library (gatsbyjs#8686)
  updated gatsby-plugin-remove-trailing-slashes docs to include link re… (gatsbyjs#8720)
  feat(www/starters): Add TypeScript and Contentful starter (gatsbyjs#8704)
  fix(docs): fix typos in template doc (gatsbyjs#8692)
  feat(www): change chevron direction on sticky responsive sidebar (gatsbyjs#8664)
  chore(release): Publish
  [integration] [cypress-gatsby] feat: Add Gatsby API support to our Cypress plugin (gatsbyjs#8641)
  chore(release): Publish
  fix(gatsby): add mjs config to webpack and resolve correctly (gatsbyjs#8717)
  feat(gatsby-plugin-netlify): add force option to createRedirect (gatsbyjs#8521)
  [www/starters] tweaks to fix DX for no GH token on www (gatsbyjs#8718)
  chore(release): Publish
  fix(gatsby-dev-cli): infer correct prefix from package path (gatsbyjs#8683)
  fix(www/starters): fix broken link to submission instructions (gatsbyjs#8715)
  chore(release): Publish
  feat(gatsby) : add createContentDigest helper (gatsbyjs#8687)
  feat: publish SendGrid case study blogpost (gatsbyjs#8592)
  ...
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.

2 participants