-
Notifications
You must be signed in to change notification settings - Fork 16
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
fix: update react-scripts and cli-style at same time to use eslint 7 #475
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Actually, react-scripts @ 4 might work for us: https://github.com/facebook/create-react-app/releases/tag/v4.0.0
|
31a0b7b
to
6acb79a
Compare
Since ESLint changed their plugin resolution mechanisms for ESLint 6 and after upgrading to ESLint 6 in cli-style 7, this changes fixes the problem where `cli-app-scripts build` fails for apps that use cli-style. Since we manually extend the existing ESLint configuration from the project in the shell ESLint configuration, and all dependencies are hoisted, we can avoid leveraging ESLint's internal module resolution which is based on CWD (project/.d2/shell).
6acb79a
to
3000b18
Compare
3000b18
to
647b059
Compare
@amcgee I'd like to update cli-style and react-scripts to the latest versions in app-platform, and release it on |
8145035
to
eb5c9e7
Compare
@@ -8,27 +8,29 @@ | |||
}, | |||
"workspaces": { | |||
"packages": [ | |||
"examples/simple-app", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did we decide to do this in this change? I think there were some issues I tried it previously... don't remember exactly what they were
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can revert it in alpha
before we cut the proper release. The reason I went ahead with it was that I was having issues with the simple-app when I was testing out the shell changes, and this made it work.
"build": "yarn build:adapter && yarn build:example", | ||
"build:adapter": "yarn workspace @dhis2/app-adapter build", | ||
"build:example": "yarn workspace simple-app build", | ||
"install:example": "yarn workspace simple-app install --force --check-files", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we're using the workspace here I don't think the install step is necessary
## [5.4.1-alpha.1](v5.4.0...v5.4.1-alpha.1) (2020-10-30) ### Bug Fixes * update react-scripts and cli-style at same time to use eslint 7 ([#475](#475)) ([8fd9225](8fd9225))
🎉 This PR is included in version 5.4.1-alpha.1 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
## [5.4.1](v5.4.0...v5.4.1) (2020-10-30) ### Bug Fixes * update react-scripts and cli-style at same time to use eslint 7 ([#475](#475)) ([8fd9225](8fd9225))
🎉 This PR is included in version 5.4.1 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
Update to newest cli-style and react-scripts to ensure that we get the improvements made in react-scripts for the changes in eslint related to resolving plugins.
Supersedes #473