chore: add dev:million-lint command
#8032
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
|
New dependencies detected. Learn more about Socket for GitHub ↗︎
|
stipsan
force-pushed
the
add-million-lint-mode
branch
from
30be234 to
7e5974a
Compare
|
No changes to documentation |
⚡️ Editor Performance ReportUpdated Mon, 16 Dec 2024 11:45:37 GMT
Detailed information🏠 Reference resultThe performance result of
🧪 Experiment resultThe performance result of this branch
📚 Glossary
|
|
Component Testing Report Updated Dec 16, 2024 11:43 AM (UTC) ❌ Failed Tests (6) -- expand for details
|
stipsan
force-pushed
the
add-million-lint-mode
branch
from
7e5974a to
7414464
Compare
stipsan
force-pushed
the
add-million-lint-mode
branch
from
7414464 to
9d075f4
Compare
| @@ -83,3 +83,6 @@ yalc.lock | |||
| ## Documentation Report | |||
| scripts/docs-report.md | |||
|
|
|||
| # Temporary data collected by Million Lint | |||
| **/.million/store.json | |||
| dev/test-studio/.react-compiler-bailout-report.json | |||
There was a problem hiding this comment.
Committing this to git would cause all sorts of annoying merge conflicts on PRs that do perf work, that for example makes components no longer bail out. Or when eslint-plugin-react-compiler and babel-plugin-react-compiler itself changes what cases it bails and supports as we bump deps.
| ? { | ||
| target: '18', | ||
| sources: (filename) => { | ||
| if (filename.includes('node_modules')) { |
There was a problem hiding this comment.
This is the default behavior of sources, since we're overriding it we have to ensure we don't accidentally try running the compiler on non-src files from npm.
There was a problem hiding this comment.
Would love to have this as a code comment too 😇
| vite(viteConfig: UserConfig): UserConfig { | ||
| const reactProductionProfiling = process.env.REACT_PRODUCTION_PROFILING === 'true' | ||
|
|
||
| return { | ||
| ...viteConfig, | ||
| plugins: millionLintEnabled | ||
| ? [ | ||
| require('@million/lint').vite({ |
There was a problem hiding this comment.
I tried to use it as a regular import at the top originally.
But for some reason, if you have this line top-level:
import Million from '@million/lint'then sanity build hangs forever
There was a problem hiding this comment.
Any idea why that might be? This is very useful context, btw - would you mind adding that to a code comment here so it doesn't get lost for the next reader?
There was a problem hiding this comment.
Yep, I'll add a comment. It's the WebSocket server setup for the VS Code extension that isn't closed 🙌
| @@ -17,7 +17,8 @@ | |||
| "check:deps": "pnpm --recursive --parallel exec depcheck", | |||
| "check:format": "prettier . --check", | |||
| "check:lint": "turbo run lint --continue -- --quiet", | |||
| "check:react-compiler": "eslint --no-inline-config --no-eslintrc --ext .cjs,.mjs,.js,.jsx,.ts,.tsx --parser @typescript-eslint/parser --plugin react-compiler --rule 'react-compiler/react-compiler: [warn]' --ignore-path .eslintignore.react-compiler --max-warnings 79 .", | |||
| "check:react-compiler": "eslint --cache --no-inline-config --no-eslintrc --ext .cjs,.mjs,.js,.jsx,.ts,.tsx --parser @typescript-eslint/parser --plugin react-compiler --rule 'react-compiler/react-compiler: [warn]' --ignore-path .eslintignore.react-compiler --max-warnings 77 .", | |||
| "report:react-compiler-bailout": "eslint --cache --no-inline-config --no-eslintrc --ext .cjs,.mjs,.js,.jsx,.ts,.tsx --parser @typescript-eslint/parser --plugin react-compiler --rule 'react-compiler/react-compiler: [error,{__unstable_donotuse_reportAllBailouts:true}]' --ignore-path .eslintignore.react-compiler -f ./scripts/reactCompilerBailouts.cjs . || true", | |||
There was a problem hiding this comment.
The reason this command has __unstable_donotuse_reportAllBailouts:true is because we want to make sure Million Lint runs on all components that are not optimized by the compiler.
Running the eslint compiler plugin reports 77 issues atm that it considers actionable.
Actionable means its bailing because of a problem with the code, for example reading a ref.current during render (which has always been unsafe, and the reason it's not impacting us atm is because we're over-rendering).
There are a lot of cases where the compiler skips a component that's valid. For example it doesn't fully support TypeScript "satisfies" syntax, here it bails, while here it works.
These type of issues aren't flagged without the __unstable_donotuse_reportAllBailouts option to avoid leading people to refactor code in ways to only please the current version of the compiler, instead of what's best for code readability and for the humans working in the project.
In this case we don't really care or show the bailed out components that aren't actionable, we just gobble it up in a JSON file to setup mutually exclusive source filters between Million Lint and React Compiler.
That way we get intel on which of the components that don't have react errors, but for whatever reason isn't compiled, is worth looking into manually memoizing in the mean time.
stipsan
force-pushed
the
add-million-lint-mode
branch
from
9d075f4 to
f5cd7df
Compare
Description
This PR sets up a script that run Million Lint on the Test Studio locally:
million.mov
There's a lot to say on this topic, and if performance work interests you and you have a hot beverage ready, expand the
Verbose informationtoggle at the end of the PR. The PR will otherwise be as brief and easy to review as possible 🙌With Million Lint we can:
useEffect.What to review
Have a look on the diff, as I'm calling out what to review as my own review comments.
Please mark them as resolved once you've seen them, and feel free to ask direct follow up questions there and then resolve once you're satisifed with my answer 😌
Testing
Until Million supports running in production you'll only able to run it locally, and AFAIK the IDE extension is only available on VS Code for now.
git clone,pnpm installand runpnpm dev:million-lint.You can look at
dev/test-studio/.react-compiler-bailout-report.jsonif you wonder exactly what files are instrumented and will have intellisense available.Why can't we just run Million on every file?
Since we currently have to disable React Compiler if we want to run Million on everything, it significantly increases renders and thus render data.
This isn't the first time we've had a problem like this. Our very own @runeb has official React contributor status after he sent a fix that allowed us to run the React DevTools profiler on more than just a few seconds in the Studio.
On an M1 Pro 32GB RAM MacBook Pro you'll barely able to view a simple document list in Structure Tool, open a document, or type to fast in the search input and it dies.
On a M4 Max 128GB RAM MacBook Pro you get quite a bit further, and can type into fields for a few minutes before it caves. Typing into a Portable Text field kills it almost instantly.
Notes for release
N/A this is purely a monorepo addition, only to the test studio. It doesn't affect what we publish to npm with
sanityor@sanity/visionin any way.Verbose information
How Million and React Scan compares to React DevTools
Million builds on top of React Scan, and integrates its performance profiling data with VS Code.
The benefit of React Scan, over the regular React Profiler in React DevTools, is that it instruments and traces how often hooks, props and state change over time and how it leads to cascading rerenders.
React DevTools can tell you what happened on each render frame, and is enough when you deal with performance issues where a single render is very slow.
In our codebase we have very few problems like that (they happen at scale, such as very large documents, large amount of fields, massive amount of editors working in the same document, huge Portable Text fields, those kind of scenarios). The general jank and slowness in the Studio is due to the "death of a thousand cuts". We have constant, cascading renders, paired with memoization that triggers garbage collection events as the memoization can't survive unstable props, state and context values. React DevTools isn't well equipped to help us here, what we need is the ability to track changes and counters over time in order to find the needles in our haystack.