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

react-stripe-elements must be installed when it isn't needed #400

Open
aldeed opened this issue Feb 27, 2019 · 2 comments
Open

react-stripe-elements must be installed when it isn't needed #400

aldeed opened this issue Feb 27, 2019 · 2 comments
Labels
bug For issues that describe a defect or regression in the released software good first issue For issues that a new contributor could likely submit a pull request for without needing much help

Comments

@aldeed
Copy link
Contributor

aldeed commented Feb 27, 2019

Type: minor

Describe the bug
The react-stripe-elements dependency is currently needed even when not using any Stripe components.

To Reproduce

  • npx create-react-app my-app
  • cd my-app
  • yarn add prop-types styled-components@3 reacto-form @reactioncommerce/components-context @reactioncommerce/components
  • yarn add --dev react-app-rewired

Then, in package.json, update the start, build, and test scripts to replace react-scripts with react-app-rewired:

"scripts": {
    "start": "react-app-rewired start",
    "build": "react-app-rewired build",
    "test": "react-app-rewired test",
    "eject": "react-scripts eject"
},

Finally, paste this into a file in the project root directory named config-overrides.js:

module.exports = function override(webpackConfig) {
  webpackConfig.module.rules.push({
    test: /\.mjs$/,
    include: /node_modules/,
    type: "javascript/auto"
  });

  return webpackConfig;
}

Then add the following at the top of App.js:

import InPageMenuItem from "@reactioncommerce/components/InPageMenuItem/v1";

Run yarn start and observe the error about the missing dependency.

Expected behavior
You should only need to install react-stripe-elements when using the Stripe components that require it.

Suggested fix
Look at our imports and try to be more specific about where that dependency gets pulled in. For example, if it's imported in utils and lots of other things import /utils rather than a specific util, that is likely the issue.

@aldeed aldeed added bug For issues that describe a defect or regression in the released software good first issue For issues that a new contributor could likely submit a pull request for without needing much help labels Feb 27, 2019
@samkelleher
Copy link

Ping: Is there any progress on this. We still ship the stripe elements even though we don't use.

@aldeed
Copy link
Contributor Author

aldeed commented Aug 22, 2019

I tried some things months ago, but no luck. I thought publishing as ES modules would resolve, but it did not seem to matter. Feel free to try to fix and PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug For issues that describe a defect or regression in the released software good first issue For issues that a new contributor could likely submit a pull request for without needing much help
Projects
None yet
Development

No branches or pull requests

2 participants