-
Notifications
You must be signed in to change notification settings - Fork 26.8k
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: handle jsxspreadattribute in inline-script-id eslint rule #32421
fix: handle jsxspreadattribute in inline-script-id eslint rule #32421
Conversation
This comment has been minimized.
This comment has been minimized.
is this waiting on anything on my end? |
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.
Looks good, thanks for the PR!
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Stats from current PRDefault Build (Decrease detected ✓)General Overall decrease ✓
Page Load Tests Overall decrease
|
vercel/next.js canary | stefanprobst/next.js fix/eslint-rule-inline-script-id | Change | |
---|---|---|---|
/ failed reqs | 0 | 0 | ✓ |
/ total time (seconds) | 3.894 | 4.131 | |
/ avg req/sec | 642 | 605.24 | |
/error-in-render failed reqs | 0 | 0 | ✓ |
/error-in-render total time (seconds) | 2.071 | 2.075 | 0 |
/error-in-render avg req/sec | 1206.92 | 1204.98 |
Client Bundles (main, webpack, commons)
vercel/next.js canary | stefanprobst/next.js fix/eslint-rule-inline-script-id | Change | |
---|---|---|---|
450.HASH.js gzip | 179 B | 179 B | ✓ |
framework-HASH.js gzip | 42.2 kB | 42.2 kB | ✓ |
main-HASH.js gzip | 27.3 kB | 27.3 kB | ✓ |
webpack-HASH.js gzip | 1.44 kB | 1.44 kB | ✓ |
Overall change | 71.2 kB | 71.2 kB | ✓ |
Legacy Client Bundles (polyfills)
vercel/next.js canary | stefanprobst/next.js fix/eslint-rule-inline-script-id | Change | |
---|---|---|---|
polyfills-HASH.js gzip | 31 kB | 31 kB | ✓ |
Overall change | 31 kB | 31 kB | ✓ |
Client Pages
vercel/next.js canary | stefanprobst/next.js fix/eslint-rule-inline-script-id | Change | |
---|---|---|---|
_app-HASH.js gzip | 1.37 kB | 1.37 kB | ✓ |
_error-HASH.js gzip | 194 B | 194 B | ✓ |
amp-HASH.js gzip | 312 B | 312 B | ✓ |
css-HASH.js gzip | 326 B | 326 B | ✓ |
dynamic-HASH.js gzip | 2.37 kB | 2.37 kB | ✓ |
head-HASH.js gzip | 350 B | 350 B | ✓ |
hooks-HASH.js gzip | 919 B | 919 B | ✓ |
image-HASH.js gzip | 4.94 kB | 4.94 kB | ✓ |
index-HASH.js gzip | 263 B | 263 B | ✓ |
link-HASH.js gzip | 2.19 kB | 2.19 kB | ✓ |
routerDirect..HASH.js gzip | 321 B | 321 B | ✓ |
script-HASH.js gzip | 383 B | 383 B | ✓ |
withRouter-HASH.js gzip | 318 B | 318 B | ✓ |
85e02e95b279..7e3.css gzip | 107 B | 107 B | ✓ |
Overall change | 14.3 kB | 14.3 kB | ✓ |
Client Build Manifests
vercel/next.js canary | stefanprobst/next.js fix/eslint-rule-inline-script-id | Change | |
---|---|---|---|
_buildManifest.js gzip | 459 B | 459 B | ✓ |
Overall change | 459 B | 459 B | ✓ |
Rendered Page Sizes
vercel/next.js canary | stefanprobst/next.js fix/eslint-rule-inline-script-id | Change | |
---|---|---|---|
index.html gzip | 531 B | 531 B | ✓ |
link.html gzip | 545 B | 545 B | ✓ |
withRouter.html gzip | 526 B | 526 B | ✓ |
Overall change | 1.6 kB | 1.6 kB | ✓ |
Default Build with SWC (Decrease detected ✓)
General Overall decrease ✓
vercel/next.js canary | stefanprobst/next.js fix/eslint-rule-inline-script-id | Change | |
---|---|---|---|
buildDuration | 20.8s | 20.9s | |
buildDurationCached | 4.2s | 4.3s | |
nodeModulesSize | 359 MB | 359 MB | -7 B |
Page Load Tests Overall decrease ⚠️
vercel/next.js canary | stefanprobst/next.js fix/eslint-rule-inline-script-id | Change | |
---|---|---|---|
/ failed reqs | 0 | 0 | ✓ |
/ total time (seconds) | 4.065 | 4.053 | -0.01 |
/ avg req/sec | 614.95 | 616.81 | +1.86 |
/error-in-render failed reqs | 0 | 0 | ✓ |
/error-in-render total time (seconds) | 2.055 | 2.163 | |
/error-in-render avg req/sec | 1216.83 | 1156.05 |
Client Bundles (main, webpack, commons)
vercel/next.js canary | stefanprobst/next.js fix/eslint-rule-inline-script-id | Change | |
---|---|---|---|
450.HASH.js gzip | 179 B | 179 B | ✓ |
framework-HASH.js gzip | 42.3 kB | 42.3 kB | ✓ |
main-HASH.js gzip | 27.5 kB | 27.5 kB | ✓ |
webpack-HASH.js gzip | 1.44 kB | 1.44 kB | ✓ |
Overall change | 71.4 kB | 71.4 kB | ✓ |
Legacy Client Bundles (polyfills)
vercel/next.js canary | stefanprobst/next.js fix/eslint-rule-inline-script-id | Change | |
---|---|---|---|
polyfills-HASH.js gzip | 31 kB | 31 kB | ✓ |
Overall change | 31 kB | 31 kB | ✓ |
Client Pages
vercel/next.js canary | stefanprobst/next.js fix/eslint-rule-inline-script-id | Change | |
---|---|---|---|
_app-HASH.js gzip | 1.35 kB | 1.35 kB | ✓ |
_error-HASH.js gzip | 180 B | 180 B | ✓ |
amp-HASH.js gzip | 305 B | 305 B | ✓ |
css-HASH.js gzip | 321 B | 321 B | ✓ |
dynamic-HASH.js gzip | 2.36 kB | 2.36 kB | ✓ |
head-HASH.js gzip | 342 B | 342 B | ✓ |
hooks-HASH.js gzip | 911 B | 911 B | ✓ |
image-HASH.js gzip | 4.98 kB | 4.98 kB | ✓ |
index-HASH.js gzip | 256 B | 256 B | ✓ |
link-HASH.js gzip | 2.21 kB | 2.21 kB | ✓ |
routerDirect..HASH.js gzip | 314 B | 314 B | ✓ |
script-HASH.js gzip | 375 B | 375 B | ✓ |
withRouter-HASH.js gzip | 309 B | 309 B | ✓ |
85e02e95b279..7e3.css gzip | 107 B | 107 B | ✓ |
Overall change | 14.3 kB | 14.3 kB | ✓ |
Client Build Manifests
vercel/next.js canary | stefanprobst/next.js fix/eslint-rule-inline-script-id | Change | |
---|---|---|---|
_buildManifest.js gzip | 458 B | 458 B | ✓ |
Overall change | 458 B | 458 B | ✓ |
Rendered Page Sizes
vercel/next.js canary | stefanprobst/next.js fix/eslint-rule-inline-script-id | Change | |
---|---|---|---|
index.html gzip | 530 B | 530 B | ✓ |
link.html gzip | 542 B | 542 B | ✓ |
withRouter.html gzip | 525 B | 525 B | ✓ |
Overall change | 1.6 kB | 1.6 kB | ✓ |
@stefanprobst @balazsorban44 If the assumption are not valid, I think we need to take a different strategy. The failing case I checked is the following code.
You can confirm AST of this by using https://astexplorer.net/ . |
@no-yan probably best to open a PR with the suggested improvement, or a new issue documenting the invalid assumption. thanks! |
…l#32421) fixes vercel#32178 the `inline-script-id` eslint rule crashed when encountering a `JSXSpreadAttribute`. this pr fixes that, and also handles `id` being passed via the spreaded object. ## Bug - [x] Related issues linked using `fixes #number` - [x] ~~Integration~~ Unit tests added - [ ] Errors have helpful link attached, see `contributing.md` ## Feature - [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR. - [ ] Related issues linked using `fixes #number` - [ ] Integration tests added - [ ] Documentation added - [ ] Telemetry added. In case of a feature if it's used or not. - [ ] Errors have helpful link attached, see `contributing.md` ## Documentation / Examples - [ ] Make sure the linting passes by running `yarn lint`
fixes #32178
the
inline-script-id
eslint rule crashed when encountering aJSXSpreadAttribute
. this pr fixes that, and also handlesid
being passed via the spreaded object.Bug
fixes #number
IntegrationUnit tests addedcontributing.md
Feature
fixes #number
contributing.md
Documentation / Examples
yarn lint