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 new Gatsby plugin for Inferno support #9138

Closed
wants to merge 4 commits into from

Conversation

DonnieWest
Copy link

Created as a proof of concept for #8858

You can see others who have had success with getting Inferno V6 running on Gatsby and I've also had some luck. Figured I'd essentially fork the Preact plugin and open this as a place for discussion. I'm willing to make any edits requested :)

@simonjoom
Copy link

simonjoom commented Oct 16, 2018

yes for me i should to trick a bit because in development react hot load give some problem:

in webpack.config i use a test case to remove react hot load.

Before to be able to publish a working plugin we should to have a possibility to disable the react hot load from gatsby , because it's not supported by inferno.

function getResolve() {
var res;
const _store$getState2 = store.getState(),
program = _store$getState2.program;

if((process.env.NODE_ENV)=="production")
--> 
            'react-dom/server': 'inferno-server',
            'react': 'inferno-compat',
            'react-dom': 'inferno-compat',

i used a other case for a build in test mode development;

       else  if (process.env.NODE_ENV=="inferno"){ 
      res.alias["inferno"]="inferno/dist/index.dev.esm.js";
      res.alias["inferno-create-element"]="inferno-create-element/dist/index.dev.esm.js";
      res.alias["inferno-create-class"]="inferno-create-class/dist/index.dev.esm.js";
      res.alias["inferno-hydrate"]="inferno-hydrate/dist/index.dev.esm.js";
      res.alias["inferno-shared"]="inferno-shared/dist/index.dev.esm.js";
      res.alias["inferno-vnode-flags"]="inferno-vnode-flags/dist/index.dev.esm.js";
       res.alias['react-dom/server']='inferno-server/dist/index.dev.esm.js';
       res.alias['react']='inferno-compat/dist/index.dev.esm.js';
       res.alias['react-dom']='inferno-compat/dist/index.dev.esm.js';
      }

and in babel-helper need it's what i added:

   if (process.env.NODE_ENV === `inferno`||process.env.NODE_ENV === `production`) {
   requiredPlugins.unshift(babel.createConfigItem([resolve(`babel-plugin-inferno`),{import:true,pragma: "h"}], {
    type: `plugin`
  }));
    }

fallbackPresets.push(babel.createConfigItem([resolve(`@babel/preset-react`), {
    useBuiltIns: true,
    pragma: (process.env.NODE_ENV === `inferno`||process.env.NODE_ENV === `prouction`)?"h":"React.createElement",
    development: stage === `develop`
  }], {
    type: `preset`
  }));

pragma is needed to have a correct output in dom in test dev "inferno" mode

Copy link
Contributor

@pieh pieh left a comment

Choose a reason for hiding this comment

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

Left some initial comments. I think @simonjoom have much more context here :)

@@ -0,0 +1,2 @@
/gatsby-node.js
/gatsby-browser.js
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it should be something like:

/*.js
!index.js
yarn.lock

We don't want any gatsby- built file in git (including gatsby-node)

Copy link
Author

Choose a reason for hiding this comment

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

I'll try to get to this tonight - thanks :)

@@ -0,0 +1,15 @@
exports.onCreateWebpackConfig = ({ stage, actions }) => {
// Requiring the server version of React-dom is hardcoded right now
// in the development server. So we'll just avoid loading Inferno there
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this copied from preact plugin or this is something new? I'm not sure what's the problem this check is workarounding

Copy link
Author

Choose a reason for hiding this comment

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

Yes it was. There are some problems in development with hot reloading on Inferno. This seems to fix it by excluding Inferno in dev and, I assumed, this was probably the same explanation for Preact so I left it

Copy link
Contributor

Choose a reason for hiding this comment

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

develop-html is only for generating things for basic index.html used in develop mode. There is separate develop stage that actually produce js bundle, so not sure if this changes anything for hot-reload

@DonnieWest
Copy link
Author

DonnieWest commented Oct 16, 2018

@simonjoom I'm getting around the hot reloading issue by just excluding Inferno during development like the Preact plugin does. It's definitely not an ideal solution, but I'd prefer to maintain consistency with how other plugins do things unless strictly necessary.

This was all I personally needed to do in order to switch to Inferno and seems pretty consistent with how their docs tell you to migrate - unless I'm missing something, we shouldn't have to do anything else. For instance, switching the pragma isn't needed because Inferno provides React.createElement, right?

Edit: Maybe I'm not actually fixing the hot reloading issue. I'll verify tonight and get back

@pieh
Copy link
Contributor

pieh commented Oct 17, 2018

Just a heads up - preact plugin has some problems currently #8545, so I wouldn't take how that one is doing as sure thing forward.

Allows hot module reloading locally for me and revamps the comment stating explicitly what we're doing here
@DonnieWest
Copy link
Author

@pieh noted. Everything you've mentioned should be resolved at this point :)

@simonjoom
Copy link

simonjoom commented Oct 17, 2018

@simonjoom I'm getting around the hot reloading issue by just excluding Inferno during development like the Preact plugin does. It's definitely not an ideal solution, but I'd prefer to maintain consistency with how other plugins do things unless strictly necessary.

This was all I personally needed to do in order to switch to Inferno and seems pretty consistent with how their docs tell you to migrate - unless I'm missing something, we shouldn't have to do anything else. For instance, switching the pragma isn't needed because Inferno provides React.createElement, right?

Edit: Maybe I'm not actually fixing the hot reloading issue. I'll verify tonight and get back

Hello
About development i think it s good to have an inferno development structure without hotreload.
Nota HMR work well and the window refresh is pretty fast with inferno

First because it's avoid you to deploy everytime your project to be able to reduce bug involving from inferno.
Second to have a development more near "production" but keeping the HMR and sourcemap that is great. As well have a development mode with no react working (only inferno) is great to test some library like react-transition-group (It's working even with inferno)

so for me it's why i create a new variable process.env.NODE_ENV=="inferno"

In my case i use
" npm run develop" who give me exactly the same than before hacking with gastby

"npm run inferno" give me my test production case so process.env.NODE_ENV=="inferno" here (hot reloaded avoided with this boolean) HMR working and no minifyjs

"npm run build" produce output with inferno + minifyjs inside without react

for who is motivated to create a full plugin
here my files changed:

https://github.com/simonjoom/material-gatsby/blob/f0544ed004e6a52851cc3a5dfeb7164c111fc091/packhack/gatsby/dist/utils/babel-loader-helpers.js#L36-L55

https://github.com/simonjoom/material-gatsby/blob/f0544ed004e6a52851cc3a5dfeb7164c111fc091/packhack/gatsby/dist/utils/babel-loader-helpers.js#L96-L102

Pragma is needed to turn around a bug in development mode with inferno. this bug come with preact as well in development

webpackconfig:
https://github.com/simonjoom/material-gatsby/blob/f0544ed004e6a52851cc3a5dfeb7164c111fc091/packhack/gatsby/dist/utils/webpack.config.js#L155-L167

https://github.com/simonjoom/material-gatsby/blob/f0544ed004e6a52851cc3a5dfeb7164c111fc091/packhack/gatsby/dist/utils/webpack.config.js#L372-L498

nota: there is lot of alias no needed for the plugin (i use a react-native-web structure it's why)

i use doting to setup the process env:
https://github.com/simonjoom/material-gatsby/blob/master/.env.development
https://github.com/simonjoom/material-gatsby/blob/master/.env.inferno
https://github.com/simonjoom/material-gatsby/blob/master/.env.production

so the gatsby.config need:

 require("dotenv").config({
  path: `.env.${process.env.NODE_ENV}`,
})

IN PACKAGE.JSON:
https://github.com/simonjoom/material-gatsby/blob/f0544ed004e6a52851cc3a5dfeb7164c111fc091/package.json#L132-L139

https://github.com/simonjoom/material-gatsby/blob/f0544ed004e6a52851cc3a5dfeb7164c111fc091/package.json#L21-L24

IN gatsby-browser-entry:

Something interesting to do remove the alias:
"create-react-context": directoryPath(.cache/create-react-context.js),

as inferno need the polyfill and not the react versius of context!
in gatsby-browser-entry you should change

const StaticQueryContext = React.createContext({})

too->

import createContext from 'create-react-context';
const StaticQueryContext = createContext({})

@DonnieWest
Copy link
Author

DonnieWest commented Oct 17, 2018

@simonjoom as mentioned, I agree it's not the ideal situation. This at least serves my personal use case and I don't think the loss of HMR is worth it in development

I'll let someone else (preferably from the Gatsby team) decide which route to go so we're not at a standstill here

Edit: Clarification

@pieh
Copy link
Contributor

pieh commented Oct 17, 2018

What is the problem with inferno and HMR - it straight doesn't work or it's buggy?

@simonjoom
Copy link

simonjoom commented Oct 18, 2018

What is the problem with inferno and HMR - it straight doesn't work or it's buggy?

With inferno:
HMR is working
the webpack-hot-middleware i mean work well and refresh the page after mods

But it's the react-hot-loader who cannot work with. because react-hot-loader was done for react and still they did not release a inferno-hot-loader to replace it

Gatsby use the both. react-hot-loader allow to have fully refresh component without page refresh

And again i think an inferno development is not to be on the side it's a very well feature.

I created a starter repo the gatsby in last release is boiled in
https://github.com/simonjoom/gatsby-starter-inferno-master (i just updated it today)

If i missed some hack code in this thread, look in https://github.com/simonjoom/gatsby-starter-inferno-master/tree/master/gatsbyhack/gatsby

i did a little video for what you get after deploy:
https://github.com/simonjoom/gatsby-starter-inferno-master/blob/master/video.mp4

for curious

@pieh
Copy link
Contributor

pieh commented Oct 18, 2018

Ah, that makes sense - yeah then we should just refresh page in there is no equiavalent to react-hot-loader for inferno. But definitely should use inferno for development to keep it as close to production as possible

@DonnieWest
Copy link
Author

@simonjoom @pieh sounds good then. I'll release the code here as my own package since this is all I need and let @simonjoom take this effort from here :)

@wardpeet
Copy link
Contributor

@DonnieWest @simonjoom I'll close this one as if I read correct @DonnieWest is not going to pursuit this on the gatsby repo. @simonjoom feel free to create a new PR with your own code.

You can also just publish it on your own and add it to our lovely plugin directory.
https://www.gatsbyjs.org/docs/submit-to-plugin-library/

@wardpeet wardpeet closed this Jan 10, 2019
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