Skip to content
This repository has been archived by the owner on Feb 18, 2024. It is now read-only.

@neutrinojs/react-components expects prop-types to be available in build #1457

Closed
lb- opened this issue Aug 28, 2019 · 2 comments
Closed

@neutrinojs/react-components expects prop-types to be available in build #1457

lb- opened this issue Aug 28, 2019 · 2 comments
Labels
cleanup fixed in Neutrino 9
Milestone
Neutrino 9

Comments

@lb-
Copy link

lb- commented Aug 28, 2019

Bug

  • What version of Neutrino are you using? 8.3.0
  • Are you trying to use any presets? If so, which ones, and what versions?
  • Are you using the Yarn client or the npm client? What version?
    • "@neutrinojs/airbnb": "^8.3.0",
    • "@neutrinojs/jest": "^8.3.0",
    • "@neutrinojs/react-components": "^8.3.0",
  • What version of Node.js are you using? v10.15.3
  • What operating system are you using? macOS

What did you do?

  1. yarn create @neutrinojs/project client
  2. First up, what would you like to create? Components
  3. Next, what kind of components would you like to create? React Components
  4. Would you like to add a test runner to your project? Jest
  5. Would you like to add linting to your project? Airbnb style rules
  6. cd ./client
  7. npm run build
  8. Add the build js output to a script tag to a html file <script src="/static/HelloWorld.js"></script>
  9. The html file is a template in a Django (Python) project, which already has react in the global available via window.React
  10. add the following HTML
  <script>
    document.addEventListener('DOMContentLoaded', function() {
      const component = HelloWorld.default;
      ReactDOM.render(React.createElement(component, { className: 'content' }), document.getElementById('root'));
    });
  </script>
  <body><div id="root"></div></body>
  1. Run the project

What did you expect to happen?

  • The HelloWorld react component should mount on the #root div
  • What actually happened, contrary to your expectations?
  1. require is not defined see Allow using @neutrinojs/react-components output directly in the browser #1425 - this issue is NOT about the require being needed.
  2. After I 'mocked' a basic require function to essentially return React from the global namespace, then it said 'prop-types' is not defined.

I expected that prop-types are stripped from the build because of this code, which react-components uses:
https://github.com/neutrinojs/neutrino/blob/master/packages/react/index.js#L28

Instead, I think what is happening is that that the webpack-node-externals is somehow thinking that the project needs prop-types because of its default behaviour call when used here:

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

I think the actual prop-types are being stripped, but maybe that happens AFTER the require calls are added in the build process. I do not really know enough about webpack to know for sure but if I skip that step and define my own externals via a custom preset at the end, prop-types is not required.

Work around + potential source of the problem

This code below solves the 'require' problem but as an aside, overriding the output in config.externals that is set by webpack-node-externals seems to resolve the issue of prop-types being expected to be available.

eg.

module.exports = {
  use: [
    "@neutrinojs/airbnb",
    "@neutrinojs/react-components",
    "@neutrinojs/jest",
    /** ensure that react is read from global - and webpack-node-externals is NOT used */
    neutrino => {
      neutrino.config.when(process.env.NODE_ENV === "production", config => {
        config.externals({ react: "React" });
      });
    }
  ]
};
@edmorley
Copy link
Member

Hi! Sorry for the delayed reply, and thank you for filing this issue.

I agree that prop-types should not be required in production. The code on master linked above is for Neutrino 9, whereas Neutrino 8 didn't use babel-plugin-transform-react-remove-prop-types - see the v8 release branch:
https://github.com/neutrinojs/neutrino/blob/release/v8/packages/react/index.js

ie: Stripping prop-types is a new feature in Neutrino 9, which is currently in release candidate (see #1129) and I'd recommend using it over Neutrino 8 even before the final release :-)

@edmorley edmorley added this to the Neutrino 9 milestone Sep 22, 2019
@lb-
Copy link
Author

lb- commented Sep 22, 2019

@edmorley - epic news!
thank you.

I am doing a blog post about using Neutrino and will try to upgrade my code to use v9 locally to see if it all works as expected.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
cleanup fixed in Neutrino 9
Milestone
Neutrino 9
Development

No branches or pull requests
2 participants
@edmorley @lb-