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

[1.0] Fix for gatsby develop hiccuping on gatsby-ssr.js using import #1125

Closed
wants to merge 2 commits into from
Closed

[1.0] Fix for gatsby develop hiccuping on gatsby-ssr.js using import #1125

wants to merge 2 commits into from

Conversation

mericsson
Copy link
Contributor

@KyleAMathews this doesn't feel right given what you wrote in PR 1081 but I wasn't sure how to apply your comment. This workaround seems to work, but I'm not sure if it has downstream effects. Still acquainting myself with the code but thought I'd put this for review to get more ideas / feedback and collaborate on this issue.

The issue, to restate it, is when using ES6 features that require webpack like import in gatsby-ssr.js then running gatsby develop dies a quick death.

(function (exports, require, module, __filename, __dirname) { import React from 'react'
                                                              ^^^^^^
SyntaxError: Unexpected token import
    at Object.exports.runInThisContext (vm.js:76:16)
    at Module._compile (module.js:542:28)
    at Object.Module._extensions..js (module.js:579:10)
    at Module.load (module.js:487:32)
    at tryModuleLoad (module.js:446:12)
    at Function.Module._load (module.js:438:3)
    at Module.require (module.js:497:17)
    at require (internal/module.js:20:19)
    at Object.<anonymous> (/Users/mericsson/dev/gatsby-using-redux/.cache/api-runner-ssr.js:2:15)
    at Module._compile (module.js:570:32)
    at Object.Module._extensions..js (module.js:579:10)
    at Module.load (module.js:487:32)
    at tryModuleLoad (module.js:446:12)
    at Function.Module._load (module.js:438:3)
    at Module.require (module.js:497:17)
    at require (internal/module.js:20:19)

Appreciate any help to dig into this. Thanks!

import { Provider } from 'react-redux'

import createStore from './src/state/createStore'

exports.replaceServerBodyRender = ({ component: body }) => {
exports.replaceRenderer = ({ bodyComponent, replaceBodyHTMLString }) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Cool!

@gatsbybot
Copy link
Collaborator

Deploy preview ready!

Built with commit f75845f

https://deploy-preview-1125--using-drupal.netlify.com

@gatsbybot
Copy link
Collaborator

Deploy preview ready!

Built with commit f75845f

https://deploy-preview-1125--gatsbygram.netlify.com

@gatsbybot
Copy link
Collaborator

Deploy preview ready!

Built with commit f75845f

https://deploy-preview-1125--gatsbyjs.netlify.com

@KyleAMathews
Copy link
Contributor

Yeah... this means any plugin which implements gatsby-ssr.js won't work as expected in development since they can't set their header/postbody components.

@mericsson
Copy link
Contributor Author

Ah makes sense. OK feel free to close. Not familiar enough with webpack to know where to start with your suggestion. :/

@KyleAMathews
Copy link
Contributor

The redux example changes would still be great to get in!

@mericsson
Copy link
Contributor Author

OK I'll close this and open an new one with the redux example changes.

@mericsson mericsson closed this Jun 8, 2017
@scottyeck
Copy link
Contributor

@mericsson Correct me if I'm wrong, but this was closed even though the original issue wasn't fixed - am I reading this right? If so, can we re-open? I'd love to get this fixed / fix it if need be. 😎

@mericsson
Copy link
Contributor Author

@scottyeck yep you're right. This was closed though original issue was not fixed. I'll re-open it. If you want to give a try to fix it that would be awesome!

@mericsson mericsson reopened this Jun 12, 2017
@KyleAMathews
Copy link
Contributor

haha no it should actually be closed and an issue be created for this.

The normal pattern is:

  1. create issue
  2. create PR to fix issue

if PR doesn't work, repeat with more PRs until one is merged and then close issue 1.

The problem here is there wasn't an issue ever created so closing this PR left us without something tracking the issue.

Could one of you create an issue for this and we move on from there?

@mericsson
Copy link
Contributor Author

I'll create an issue. thanks @KyleAMathews

@mericsson
Copy link
Contributor Author

Opened Issue 1156 to track this.

@scottyeck
Copy link
Contributor

Ah great - totally didn't realize this was a PR and not an issue. My bad.

Thanks all 😎

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.

4 participants