-
Notifications
You must be signed in to change notification settings - Fork 1
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
Ft/upgrade next to 15 #1001
base: main
Are you sure you want to change the base?
Ft/upgrade next to 15 #1001
Conversation
…nresolved imports for SVG files
…nder-result-naming-convention rule
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.
👍🏽
Still going through it but quick feedback on issues that kind of stood out to me.
const flatCompat = new FlatCompat(); | ||
|
||
module.exports = [ | ||
...fixupConfigRules(flatCompat.extends("plugin:@next/next/core-web-vitals")), |
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.
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.
@kilemensi I came across this issue and the only workaround I had was to use eslint compatibility utilities to fix up things.
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.
…-hooks dependency
@kilemensi this seems to work for js apps if we put eslint configs at root but since ts apps configurations have some differences, we may have to retain them as is. |
There is a slight chance I've misread it @koechkevin so please read the docs again but my understanding is as follow:
Without the flag, With the flag on, the process is actually reversed. In a monorepo, the second approach sounds like the right approach to me. |
I just saw
fly by my screen @koechkevin. Are we changing the approach? |
@kilemensi been playing around with this and it looks like even without a flag 🤔 eslint traverses backwards. So I just removed eslint configurations in apps and retained one in the root. I had to push it just to be sure that things aren't cached on my end. |
@kelvinkipruto @m453h spare some time here |
@koechkevin Just tested it on ClimateMapped, and it works fantastic. Perhaps we can now use |
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 have tested CivicSignalBlog
, ClimateMapped
and VPNManager
and they have worked , I'll need to test the remaining apps as well.
Just a thought I had as I was reading through this discussion... would it make sense to have a root-level eslint.config.js
and extend it per specific app's needs ? Or would that lead to unnecessary complexity?
…onfig and dependencies
@m453h The configs for all apps are similar, would it make sense to have as many eslint configs when only 1 at the root can work? |
Yes, I was thinking that just one |
@m453h typescript apps' configurations are slightly different from those for JS apps so we'd have to retain app-level configurations for the TS apps. |
…ross multiple packages
Description
This PR upgrades the Next.js framework from the current version to v15.0.3 to leverage on the latest features, performance improvements, and bug fixes introduced in this release.
Key Changes
.eslintignore
and.eslintrc
since with flatConfigs all can be defined in eslint.config.js.You may notice many changed files in component files, but this is due to a change in how to disable a rule in a file with flat configs.
// eslint-disable-next-line testing-library/render-result-naming-convention
inside the test file since this disable command has been specified in common eslint config"testing-library/render-result-naming-convention": "off",
.// eslint-disable-next-line react-hooks/exhaustive-deps
occurrences have now been replaced with/* eslint react-hooks/exhaustive-deps: "off" */
Review of this PR should be around
*/**/eslint.config.js
andpackages/eslint-config-commons-ui
Fixes # (issue)
Type of change
Please delete options that are not relevant.
Screenshots
Checklist: