feat: update react-scripts #721
Conversation
ghost
force-pushed
the
update-react-scripts
branch
from
87f2d99 to
ff66c24
Compare
ghost
marked this pull request as ready for review
shell/.eslintrc.js
Outdated
| const isDevelopment = !isRunningHere && hasRootConfig | ||
|
|
||
| // Use local config for developing this library, react-app preset for linting app code | ||
| const extendsList = isDevelopment ? rootConfigPath : 'react-app' |
There was a problem hiding this comment.
This was called extendsList before, but it could be a string, too. So mabye just rename it to extends? Then it matches the exported value's name
There was a problem hiding this comment.
That would be cleaner yeah, but it's a reserved word. So I kept the original name. We could rename it to something else though. Or just put the ternary in the exported object directly.
There was a problem hiding this comment.
Ah, makes sense that it's a reserved word. How about:
module.exports = {
// Use local config for developing this library, react-app preset for linting app code
ignorePatterns: isDevelopment ? [] : ['src/D2App/*'],
// Use local config for developing this library, react-app preset for linting app code
extends: isDevelopment ? rootConfigPath : 'react-app',
}There was a problem hiding this comment.
Yeah that looks simpler, changed it in 86bd3f9.
For speed. It would be a lot of work to address this. I could, but I'd have to postpone a couple other tasks (again). |
There was a problem hiding this comment.
For speed. It would be a lot of work to address this. I could, but I'd have to postpone a couple other tasks (again).
I don't think I'm the right person to judge whether this is the right call or not. I'll approve, is there a way to address adding back the plugin? And if not, how do we document this / prioritize this?
I think there's a misunderstanding. So the plugin I removed (eslint react preset) was removed because it was duplicated. Effectively it's still there, just not twice like before. So we don't have to add that back. The import rule is just disabled. Eventually it should be enabled and the violations should be addressed. Currently it doesn't really affect anything though. The import extensions rule is there for compliance with the es module spec that does not allow omitting the extension. This is not currently a thing for our tooling. |
| rules: { | ||
| 'import/extensions': 'off', | ||
| }, |
There was a problem hiding this comment.
Was wondering why we need that. Typescript?
There was a problem hiding this comment.
I can take care of it after we've dealt with the functional part of these changes. Having it not block these changes makes it a little easier for me to integrate it with other tasks.
ghost
force-pushed
the
update-react-scripts
branch
from
86bd3f9 to
29ddd6c
Compare
# [9.1.0-beta.1](v9.0.2-beta.1...v9.1.0-beta.1) (2022-06-21) ### Features * update react-scripts ([#721](#721)) ([dc1c5cb](dc1c5cb))
|
🎉 This PR is included in version 9.1.0-beta.1 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
# [10.0.0](v9.0.1...v10.0.0) (2022-07-26) ### Bug Fixes * remove engines field from pwa and adapter ([c3878f2](c3878f2)) * remove lint step from publish step requirements ([#695](#695)) ([a04f8f7](a04f8f7)) ### chore * drop support for node 12 ([937e5e2](937e5e2)) ### Features * update react-scripts ([#721](#721)) ([dc1c5cb](dc1c5cb)) ### BREAKING CHANGES * dropped support for node 12. The platform now requires node 14+.
|
🎉 This PR is included in version 10.0.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
See: https://dhis2.atlassian.net/browse/CLI-73.
After the upgrade I was running into this issue: facebook/create-react-app#12177. I've rebuilt the lockfile, that has addressed it. It does mean that everything has been updated to latest though (within the package.json version ranges). If all updated packages properly followed semver that should be fine, but we should double check to make sure. There might also be less drastic ways of addressing the issue.
I've disabled the import extensions rule for now. Besides that I've clarified the logic in the shell eslint config to what made it a little easier to read for me. In that config I also removed the react plugin for the local linting, because that plugin is already present in the cli style config, which caused eslint to fail with a warning.
Changelog for the move from 4 to 5: https://github.com/facebook/create-react-app/blob/main/CHANGELOG.md#500-2021-12-14