neutrino icon indicating copy to clipboard operation
neutrino copied to clipboard

Allow using @neutrinojs/react-components output directly in the browser

Open ribosradagast opened this issue 5 years ago • 8 comments

Bug or issue? Bug

Please try to answer the following questions:

  • What version of Neutrino are you using? 8.3.0
  • Are you trying to use any presets? If so, which ones, and what versions? @neutrinojs/react-components
  • Are you using the Yarn client or the npm client? What version? npm 6.4.1
  • What version of Node.js are you using? v10.13.0
  • What operating system are you using? centos7
  • What did you do? made a basic app with a single component in the src/components/ComponentName/index.jsx file with the following contents:
import React from 'react';

function ComponentName() {
return (
<div> stuff</div>
);
}
export default ComponentName;
  • What did you expect to happen? Expected npm build to create a usable webpacked ComponentName.js
  • What actually happened, contrary to your expectations? the webpacked file in build/ComponentName.js containes a line a.exports=require('react') which crashes the browser when used in a

Feature request or enhancement?

Bug report

ribosradagast avatar Jun 26 '19 21:06 ribosradagast

You probalby get your answer from the neutrino website: react-components

Requirements

Node.js v6 LTS, v8, v9 Yarn v1.2.1+, or npm v5.4+ Neutrino v8 React, React DOM Installation

@neutrinojs/react-components can be installed via the Yarn or npm clients. Inside your project, make sure neutrino and @neutrinojs/react-components are development dependencies. You will also need React and React DOM for actual component development.

The react-components have the react modules only with npm dev-mode. The actual build is without the react modules and must be provided separately.

Tassfighter avatar Jul 11 '19 09:07 Tassfighter

@ribosradagast - I ran into a very similar issue and after about a week of chipping away at it I think I have a solution.

I do think there is a problem with the react-components or at least some docs that are not super clear that could be improved. Prop-types are also required by the basic build, even though they should be stripped. My code below appears to fix this also, but I am not sure why.

File: .neutrinorc.js

module.exports = {
  use: [
    "@neutrinojs/airbnb",
    "@neutrinojs/react-components",
    "@neutrinojs/jest",
    neutrino => {
      neutrino.config.when(process.env.NODE_ENV === "production", config => {
        config.externals({
          react: {
            amd: "react",
            commonjs: "react",
            commonjs2: "react",
            root: "React"
          }
        });
      });
    }
  ]
};

Essentially what we are doing is adding a final step that overrides the setting of externals, which webpack uses to map dependencies that should NOT be bundled.

Notes

  • The assumption here is that React will be available in the root (window) of your side and called React not react, but just change the value under root in the code above if it is different.
  • This will automatically bundle ALL other dependencies, except prop types, you will have to now 'black-list' any requirements you do not want bundled in that same 'externals' object.
  • There appears to be a related issue where prop-types are also expected (you will encounter this if you solved the require problem), but the code above somehow seems to make that work and proptypes are correctly removed.
  • Webpack requires the values for amd, commonjs etc - even though the UMD build used by this preset only uses the 'root' one.

Detailed notes

The react-component preset sets externals to an empty object by default here. https://github.com/neutrinojs/neutrino/blob/master/packages/react-components/index.js#L14

Then if the object exists, in production, it will use a package called webpack-node-externals to set up ALL external dependencies (of which React is part of) as external and not bundle them. That happens here.

https://github.com/neutrinojs/neutrino/blob/master/packages/react-components/index.js#L63

However, that package is not really designed for output to web / UMD (ie. setting and using globals), as explained on this issue.

https://github.com/liady/webpack-node-externals/issues/17

So... by setting the externals ourselves, we override the usage of webpack-node-externals and instead follow the webpack usage as documented here.

https://webpack.js.org/configuration/externals/#externals

Finally, the build that does get produced, is a UMD build which will use the 'root' version of the external project if present (note the upper case React, as the website I am using sets a global of React not react).

Additional to this, there is code that removes prop-types in the react preset here: https://github.com/neutrinojs/neutrino/blob/master/packages/react/index.js#L28

I am not sure why but the basic out of the box usage of react-components still seems to require prop-types. Maybe this needs a stand-alone bug.

lb- avatar Aug 28 '19 11:08 lb-

Update, the last line in the use array can just be:

    neutrino => {
      neutrino.config.when(process.env.NODE_ENV === "production", config => {
        config.externals({ react: "React" });
      });
    }

lb- avatar Aug 28 '19 20:08 lb-

Hi! Yeah the @neutrinojs/react-components preset currently presumes the output is going to be consumed by a project using a bundler such as webpack, rather than direct browser usage.

This is definitely something that the docs could make clearer and/or perhaps some additional options added to the preset to save people from having to change a bunch of settings themselves. I'm not too familiar with what would need to be changed, but if you come up with a list of settings changes we could take a look at including this in the preset under an option :-)

edmorley avatar Sep 22 '19 20:09 edmorley

@edmorley - thanks for the response, I think there needs to be an option for externals that does not use the webpack-node-externals but instead turns that off and just submits the config.externals with the object supplied.

eg. useRawExternals which defaults to false, which will in turn mean the existing behaviour is preserved.

I have got some code example for changes to the react-components but I think it might be best to actually change this in the library code and somehow just use that as is in the react-components one.

Example implementation.

       # https://github.com/neutrinojs/neutrino/blob/master/packages/react-components/index.js#L14
      externals: opts.externals !== false && {},
      useRawExternals: false, // added default for usage of options.externals here
        # https://github.com/neutrinojs/neutrino/blob/master/packages/react-components/index.js#L62
        .when(options.externals, config =>
          config.externals([options.useRawExternals ?  options.externals: nodeExternals(options.externals)]),
        )

On top of this, it might be nice to be explicit about how externals is used in the react-components readme. Similar to how the 'presets' section explains this in the library readme.

https://github.com/neutrinojs/neutrino/blob/master/packages/library/README.md#preset-options

I know that the react-components one explains that it inherits library but this might be worth repeating.

I am happy to do a PR if this is something useful, any suggestions on the option name would be appreciated also.

lb- avatar Sep 22 '19 23:09 lb-

Is there any update on the status of this? I'm attempting to find something to make development much easier for making React components to be added to a pre-existing, non-webpacked, site.
Neutrino react-components looks perfect to me, except for the fact the output js files have require("react") in them

For reference I'd love to be able to use the steps listed here to add React slowly to the site. https://reactjs.org/docs/add-react-to-a-website.html#add-react-in-one-minute

MattFellows avatar Nov 11 '20 09:11 MattFellows

Update, the last line in the use array can just be:

    neutrino => {
      neutrino.config.when(process.env.NODE_ENV === "production", config => {
        config.externals({ react: "React" });
      });
    }

Does it work as a temporary workaround? If yes we could use it as a base for an internal option Checking of environment process.env.NODE_ENV === "production" may be not necessary. Please try also without it.

constgen avatar Nov 12 '20 08:11 constgen

You can override the settings like so to get this working:

use: [
  ...
  reactComponents(),
  neutrino => {
         neutrino.config.output
            .library('MyComponents')
            .libraryTarget('window');
         neutrino.config.externals({
            react: 'React'
         });
      }

Which can be used with something like this (React 18 syntax):

(function() {
      const container = document.getElementById('react-test');
      const root = ReactDOM.createRoot(container);
      root.render(React.createElement(MyComponents.App));
   })();

Which binds the App component to #react-test.

alalonde avatar Apr 27 '22 20:04 alalonde