create-react-app
create-react-app copied to clipboard
facebook
fix: exclude .spec and .test files from type checking
This PR fixes react-scripts@5 including test files when type checking during npm start and npm build. This was changed in the upgrade to webpack@5 https://github.com/facebook/create-react-app/pull/11201/files#diff-8e25c4f6f592c1fcfc38f0d43d62cbd68399f44f494c1b60f0cf9ccd7344d697R732 however the syntax does not exclude .spec and .test files.
There is a reproduction repo at https://github.com/jwalton9/cra-test-type-error which introduces a type error to src/App.test.tsx. Running npm start causes these issues to be reported.
The changes were validated by using npm link and running npm start
Fixes #11979
Hi @jwalton9!
Thank you for your pull request and welcome to our community.
Action Required
In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.
Process
In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.
Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.
If you have received this in error or have any questions, please contact us at [email protected]. Thanks!
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!
Is there possibly an ETA on when this will be merged? This is a pretty sticky issue for us in trying to upgrade from 4 to 5.
As a temporary work around, if you are using Craco, you can use this configuration file to inject the exclude without the typo in it:
// Taken from the original webpack config
const ForkTsCheckerWebpackPlugin =
process.env.TSC_COMPILE_ON_ERROR === 'true'
? require('react-dev-utils/ForkTsCheckerWarningWebpackPlugin')
: require('react-dev-utils/ForkTsCheckerWebpackPlugin');
module.exports = {
webpack: {
configure: (webpackConfig, {env, paths}) => {
webpackConfig.plugins.map((plugin) => {
if (plugin instanceof ForkTsCheckerWebpackPlugin) {
plugin.options.issue.exclude.push({file: '**/src/**/?(*.){spec,test}.*'});
}
return plugin;
})
return webpackConfig;
},
},
};
@iansu @mrmckeb Any chance this could be reviewed soon or at least an update on when we can expect this to be reviewed? Seems like a pretty simple fix, and I'd like to remove our custom config to get around this issue
Please can this be merged. I've never seen a 1 character change PR that is so impactful sit idle for so long.
@iansu @mrmckeb this change seems to be impactful for a number of users (myself included) and it has been sitting idle for close to a year. IMO this should be merged or closed with directions for a workaround.
@iansu @mrmckeb +1 to what @tonyjmnz said. I'm surprised by the lack of communication on this issue. I may be misunderstanding, but this one-character change seems to be fixing what amounts to a typo.
@jwalton9 how come this isn't reviewed at all? Did you manage to find anything else regarding this issue? I do understand that comma is interpreted as a literal character, rather than as a separator between two options and that's why they used | instead, however changing it manually seems to fix the issue for me too
Was there any announcement, post, github issue where they explain what's going on with CRA? No release since April 2022 and PRs aren't getting approved. I see people wanting to contribute to the project, but they get no support from the code owners.
Here we have a very simple PR that would solve an annoying issue and it def shouldn't take one year to get it MERGED. Not approved, MERGED,
@Mboulianne I think you read https://github.com/facebook/create-react-app/discussions/11768
