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

Incorrect behaviour when using Jest's Array type config (collectCoverageFrom) #240

Closed
hardyscc opened this issue May 8, 2018 · 5 comments

Comments

@hardyscc
Copy link

hardyscc commented May 8, 2018

I don't know whether this is the expected behaviour or not, however i just report this in case someone face the same problem..

in my package.json

  "jest": {
    "collectCoverageFrom": [
      "src/components/**/*.{js,jsx}"
    ]
  }

after apply react-app-rewired, the collectCoverageFrom become [ 'src/components/**/*.{js,jsx}', 'src/**/*.{js,jsx,mjs}' ], which is not the default behaviour of react-scripts, as it won't include the jest default 'src/**/*.{js,jsx,mjs}' .

@dawnmist
Copy link
Collaborator

dawnmist commented May 9, 2018

This has occurred due to a combination of two things.

First - most jest configuration options are not permitted by react-scripts. How react-app-rewired works around this is it takes a copy of all your defined jest options from package.json, and then passes into react-scripts everything except your jest options (i.e. it deletes the jest options part). This means that react-scripts isn't seeing the overrides for even the few fields it does permit overrides for.

React-app-rewired then merges any array or object values defined in package.json with the original values from react-scripts instead of overwriting them. I don't know the history of why it was done this way rather than overriding them.

What you can do to make an override setting instead of a setting to be merged is to apply the setting inside the jest function in config-overrides, which is run after the merge has occurred. This lets you make a full replacement instead of a merge replacement.

module.exports = {
  jest: (config) => {
    config.collectCoverageFrom = ["src/components/**/*.{js,jsx}"];
    return config;
  }
}

@hardyscc
Copy link
Author

hardyscc commented May 9, 2018

Thank you very much, IMO the purpose of react-app-rewired is to temporary modify the unsupported config, and remove it once react-scripts is supported.. so I think it shouldn't break the default behaviour of any untouched config unless it has a bigger reason.

@Gidgidonihah
Copy link
Contributor

The response to fix it in config-overrides.js is super helpful. Thanks, I wasted a LOT of time trying to figured out why this wasn't working the node_modules.

However, I would argue that this issue shouldn't be closed. Either revisiting the decision to merge, or at the very least comments in the readme about it.

I've opened #294 to add comments to the readme, but currently the travis builds are failing.

@akuji1993
Copy link

Anyone has tried this in the last few weeks? collectCoverageFrom doesn't seem to be impacted by this fix, as it still shows it tested ALL qualifying files with the correct ending, even though they are not located in a folder that is included in the array.

@akuji1993
Copy link

akuji1993 commented Apr 29, 2019

Found another solution, for this specific command, you can just add it to your cli command in package.json like so
"test": "react-app-rewired test --env=jsdom --collectCoverageFrom=src/components/**/*.{ts,tsx}"

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

No branches or pull requests

5 participants