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

feat(npm/react): allow custom webpack config path for react-scripts plugin #16644

Merged
merged 2 commits into from
May 28, 2021

Conversation

pashidlos
Copy link
Contributor

@pashidlos pashidlos commented May 24, 2021

User facing changelog

react-scripts plugin now could be configured with custom webpack config path

const injectDevServer = require('@cypress/react/plugins/react-scripts')
const path = require('path')

module.exports = (on, config) => {
  injectDevServer(
    on, 
    config, { 
      webpackConfigPath: './your/custom/path/webpack.config.js'  // <- added this option
    })

  return config
}

PR Tasks

  • Has the original issue or this PR been tagged with a release in ZenHub?

@cypress-bot
Copy link
Contributor

cypress-bot bot commented May 24, 2021

Thanks for taking the time to open a PR!

@pashidlos pashidlos changed the title 16480 custom webpack path feat: react-plugins. allow custom webpack config path May 24, 2021
@pashidlos pashidlos marked this pull request as draft May 24, 2021 08:19
@pashidlos pashidlos force-pushed the 16480-custom-webpack-path branch 13 times, most recently from 753d233 to 1b1270b Compare May 24, 2021 14:28
@pashidlos pashidlos marked this pull request as ready for review May 24, 2021 15:23
@lmiller1990 lmiller1990 self-requested a review May 25, 2021 01:20
Copy link
Contributor

@lmiller1990 lmiller1990 left a comment

Choose a reason for hiding this comment

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

Code seems generally fine. Two comments:

  • have you tested this? I see we have not actually added a test. I wonder if we could add a simple unit test, perhaps? I can probably look into this if you aren't sure how or don't have the time. You'd just write it with mocha - two use cases would be loading w/ default location, and loading w/ a custom location.
  • if you want this to go out asap, it'll need to target master. Otherwise it will need to wait for the next release (every 2 weeks).

Please let me know

  1. do you plan to retarget for master, or happy to wait for 2 weeks?
  2. about adding a unit test - can you do this, or do you need me to look into this?
  3. have you actually tested this in some capacity, or should I do a manually QA on top of the unit test we should add before merging?

Thanks!

@lmiller1990 lmiller1990 self-assigned this May 25, 2021
@lmiller1990 lmiller1990 added the npm: @cypress/react @cypress/react package issues label May 25, 2021
@pashidlos pashidlos changed the base branch from develop to master May 25, 2021 07:33
@pashidlos
Copy link
Contributor Author

@lmiller1990 thanks for quick response

  • do you plan to retarget for master, or happy to wait for 2 weeks?

retargeted to master, unfortunately passing pipeline is a challenge due to unstable tests

  • about adding a unit test - can you do this, or do you need me to look into this?

I wasn't able to find any example of those (should I add new npm script?)
please, assist with unit tests

  • have you actually tested this in some capacity, or should I do a manually QA on top of the unit test we should add before merging?

I checked it manually and the default behaviour is covered with examples projects AFAIK

@lmiller1990 lmiller1990 self-requested a review May 28, 2021 06:07
@lmiller1990 lmiller1990 changed the title feat: react-plugins. allow custom webpack config path feat(npm/react): allow custom webpack config path for react-scripts plugin May 28, 2021
@lmiller1990 lmiller1990 merged commit c618d30 into cypress-io:master May 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
npm: @cypress/react @cypress/react package issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Component tests. plugins/react-scripts. Allow configure of webpack config path
2 participants