fix: replace usage of `fromEntries` in browser bundled `resolve-rewrites.ts` #25208

jamsinclair · 2021-05-18T02:20:12Z

Bug

Related issues linked using fixes #number
Integration tests added

Currently rewritten routes that use a has condition throw an Object.fromEntries is not a function error in older browsers.

Documentation / Examples

Make sure the linting passes

Solution

As mentioned in issue #25207, looks like last year the team decided not to include the fromEntries polyfill #15772 (comment).

As such, we should avoid using non-polyfilled methods in core next.js code bundled in the browser.

This PR's changes should result in the same object being returned, without using fromEntries.

timneutkens

Looks fine to land, however a test for IE11 for this behavior would be good to add (in a follow-up PR) 👍 Thanks!

jamsinclair · 2021-05-18T10:38:20Z

@timneutkens Thank you! 😊 I'm happy to follow up with a test. Can you point me to the appropriate directory or area to add this kind of test?

timneutkens · 2021-05-18T10:46:56Z

You can add a new directory in test/integration/ (copy from test/integration/basic or similar) and then add it to this line so that it runs on IE11: https://github.com/vercel/next.js/blob/canary/azure-pipelines.yml#L89

ijjk · 2021-05-18T20:03:05Z

Stats from current PR

Default Server Mode (Decrease detected ✓)

General Overall increase ⚠️

	vercel/next.js canary	jamsinclair/next.js fix/from-entries-rewrites	Change
buildDuration	15s	15s	⚠️ +4ms
buildDurationCached	4.5s	4.4s	-180ms
nodeModulesSize	46.7 MB	46.7 MB	⚠️ +51 B

Page Load Tests Overall decrease ⚠️

	vercel/next.js canary	jamsinclair/next.js fix/from-entries-rewrites	Change
/ failed reqs	0	0	✓
/ total time (seconds)	2.443	2.598	⚠️ +0.15
/ avg req/sec	1023.4	962.18	⚠️ -61.22
/error-in-render failed reqs	0	0	✓
/error-in-render total time (seconds)	1.359	1.333	-0.03
/error-in-render avg req/sec	1840.13	1875.03	+34.9

Client Bundles (main, webpack, commons)

	vercel/next.js canary	jamsinclair/next.js fix/from-entries-rewrites	Change
framework-HASH.js gzip	39.3 kB	39.3 kB	✓
main-HASH.js gzip	19.4 kB	19.4 kB	✓
webpack-HASH.js gzip	994 B	994 B	✓
Overall change	59.7 kB	59.7 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	jamsinclair/next.js fix/from-entries-rewrites	Change
polyfills-HASH.js gzip	31.1 kB	31.1 kB	✓
Overall change	31.1 kB	31.1 kB	✓

Client Pages

	vercel/next.js canary	jamsinclair/next.js fix/from-entries-rewrites	Change
_app-HASH.js gzip	1.02 kB	1.02 kB	✓
_error-HASH.js gzip	3.06 kB	3.06 kB	✓
amp-HASH.js gzip	526 B	526 B	✓
css-HASH.js gzip	334 B	334 B	✓
hooks-HASH.js gzip	890 B	890 B	✓
index-HASH.js gzip	262 B	262 B	✓
link-HASH.js gzip	1.65 kB	1.65 kB	✓
routerDirect..HASH.js gzip	331 B	331 B	✓
withRouter-HASH.js gzip	329 B	329 B	✓
bb14e60e810b..30f.css gzip	125 B	125 B	✓
Overall change	8.54 kB	8.54 kB	✓

Client Build Manifests

	vercel/next.js canary	jamsinclair/next.js fix/from-entries-rewrites	Change
_buildManifest.js gzip	390 B	390 B	✓
Overall change	390 B	390 B	✓

Rendered Page Sizes

	vercel/next.js canary	jamsinclair/next.js fix/from-entries-rewrites	Change
index.html gzip	561 B	561 B	✓
link.html gzip	570 B	570 B	✓
withRouter.html gzip	558 B	558 B	✓
Overall change	1.69 kB	1.69 kB	✓

Serverless Mode

General Overall increase ⚠️

	vercel/next.js canary	jamsinclair/next.js fix/from-entries-rewrites	Change
buildDuration	17.5s	17.2s	-262ms
buildDurationCached	5.5s	5.4s	-116ms
nodeModulesSize	46.7 MB	46.7 MB	⚠️ +51 B

Client Bundles (main, webpack, commons)

	vercel/next.js canary	jamsinclair/next.js fix/from-entries-rewrites	Change
framework-HASH.js gzip	39.3 kB	39.3 kB	✓
main-HASH.js gzip	19.4 kB	19.4 kB	✓
webpack-HASH.js gzip	994 B	994 B	✓
Overall change	59.7 kB	59.7 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	jamsinclair/next.js fix/from-entries-rewrites	Change
polyfills-HASH.js gzip	31.1 kB	31.1 kB	✓
Overall change	31.1 kB	31.1 kB	✓

Client Pages

	vercel/next.js canary	jamsinclair/next.js fix/from-entries-rewrites	Change
_app-HASH.js gzip	1.02 kB	1.02 kB	✓
_error-HASH.js gzip	3.06 kB	3.06 kB	✓
amp-HASH.js gzip	526 B	526 B	✓
css-HASH.js gzip	334 B	334 B	✓
hooks-HASH.js gzip	890 B	890 B	✓
index-HASH.js gzip	262 B	262 B	✓
link-HASH.js gzip	1.65 kB	1.65 kB	✓
routerDirect..HASH.js gzip	331 B	331 B	✓
withRouter-HASH.js gzip	329 B	329 B	✓
bb14e60e810b..30f.css gzip	125 B	125 B	✓
Overall change	8.54 kB	8.54 kB	✓

Client Build Manifests

	vercel/next.js canary	jamsinclair/next.js fix/from-entries-rewrites	Change
_buildManifest.js gzip	390 B	390 B	✓
Overall change	390 B	390 B	✓

Serverless bundles

	vercel/next.js canary	jamsinclair/next.js fix/from-entries-rewrites	Change
_error.js	17 kB	17 kB	✓
404.html	2.42 kB	2.42 kB	✓
500.html	2.41 kB	2.41 kB	✓
amp.amp.html	10.8 kB	10.8 kB	✓
amp.html	1.61 kB	1.61 kB	✓
css.html	1.79 kB	1.79 kB	✓
hooks.html	1.67 kB	1.67 kB	✓
index.js	17.3 kB	17.3 kB	✓
link.js	17.6 kB	17.6 kB	✓
routerDirect.js	17.5 kB	17.5 kB	✓
withRouter.js	17.6 kB	17.6 kB	✓
Overall change	108 kB	108 kB	✓

Webpack 4 Mode (Increase detected ⚠️)

General Overall increase ⚠️

	vercel/next.js canary	jamsinclair/next.js fix/from-entries-rewrites	Change
buildDuration	12.6s	12.6s	-60ms
buildDurationCached	5.4s	5.4s	-64ms
nodeModulesSize	46.7 MB	46.7 MB	⚠️ +51 B

Page Load Tests Overall increase ✓

	vercel/next.js canary	jamsinclair/next.js fix/from-entries-rewrites	Change
/ failed reqs	0	0	✓
/ total time (seconds)	2.467	2.484	⚠️ +0.02
/ avg req/sec	1013.46	1006.25	⚠️ -7.21
/error-in-render failed reqs	0	0	✓
/error-in-render total time (seconds)	1.331	1.289	-0.04
/error-in-render avg req/sec	1877.89	1939.05	+61.16

Client Bundles (main, webpack, commons)

	vercel/next.js canary	jamsinclair/next.js fix/from-entries-rewrites	Change
677f882d2ed8..HASH.js gzip	13.3 kB	13.3 kB	✓
framework.HASH.js gzip	39 kB	39 kB	✓
main-HASH.js gzip	7.26 kB	7.26 kB	✓
webpack-HASH.js gzip	751 B	751 B	✓
Overall change	60.3 kB	60.3 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	jamsinclair/next.js fix/from-entries-rewrites	Change
polyfills-HASH.js gzip	31.3 kB	31.3 kB	✓
Overall change	31.3 kB	31.3 kB	✓

Client Pages

	vercel/next.js canary	jamsinclair/next.js fix/from-entries-rewrites	Change
_app-HASH.js gzip	1.28 kB	1.28 kB	✓
_error-HASH.js gzip	3.74 kB	3.74 kB	✓
amp-HASH.js gzip	536 B	536 B	✓
css-HASH.js gzip	339 B	339 B	✓
hooks-HASH.js gzip	887 B	887 B	✓
index-HASH.js gzip	227 B	227 B	✓
link-HASH.js gzip	1.64 kB	1.64 kB	✓
routerDirect..HASH.js gzip	303 B	303 B	✓
withRouter-HASH.js gzip	302 B	302 B	✓
e025d2764813..52f.css gzip	125 B	125 B	✓
Overall change	9.38 kB	9.38 kB	✓

Client Build Manifests

	vercel/next.js canary	jamsinclair/next.js fix/from-entries-rewrites	Change
_buildManifest.js gzip	420 B	420 B	✓
Overall change	420 B	420 B	✓

Rendered Page Sizes

	vercel/next.js canary	jamsinclair/next.js fix/from-entries-rewrites	Change
index.html gzip	614 B	614 B	✓
link.html gzip	620 B	620 B	✓
withRouter.html gzip	607 B	607 B	✓
Overall change	1.84 kB	1.84 kB	✓

Commit: dbd27d2

@timneutkens

## Summary Follow up regression test for rewrites with a `has` condition throwing errors in older browsers #25208. @timneutkens Let me know if this is what you had in mind? 😸

…tes.ts` (vercel#25208) ## Bug - [x] Related issues linked using `fixes #number` - [ ] Integration tests added Fixes vercel#25207 Currently rewritten routes that use a `has` condition throw an `Object.fromEntries is not a function` error in older browsers. ## Documentation / Examples - [x] Make sure the linting passes ## Solution As mentioned in issue vercel#25207, looks like last year the team decided not to include the `fromEntries` polyfill vercel#15772 (comment). As such, we should avoid using non-polyfilled methods in core next.js code bundled in the browser. This PR's changes should result in the same object being returned, without using `fromEntries`.

@timneutkens

## Summary Follow up regression test for rewrites with a `has` condition throwing errors in older browsers vercel#25208. @timneutkens Let me know if this is what you had in mind? 😸

fix: remove usage of fromEntries in browser bundled code

e389393

jamsinclair requested review from divmain, ijjk, lfades, shuding and timneutkens as code owners May 18, 2021 02:20

ijjk added the type: next label May 18, 2021

This comment has been minimized.

Sign in to view

timneutkens approved these changes May 18, 2021

View reviewed changes

Merge branch 'canary' into fix/from-entries-rewrites

4253d78

This comment has been minimized.

Sign in to view

Merge branch 'canary' into fix/from-entries-rewrites

dbd27d2

ijjk approved these changes May 18, 2021

View reviewed changes

kodiakhq bot merged commit fa5d41b into vercel:canary May 18, 2021

jamsinclair deleted the fix/from-entries-rewrites branch May 19, 2021 00:17

jamsinclair mentioned this pull request May 20, 2021

Follow up rewrites regression test for #25208 #25282

Merged

sync-by-unito bot mentioned this pull request May 26, 2021

chore(deps): bump next from 10.0.7 to 10.2.3 filecoin-project/slate#766

Closed

This was referenced Jul 29, 2021

[Snyk] Upgrade next from 10.0.3 to 10.2.3 bgeveritt-snyk/next.js#9

Open

[Snyk] Upgrade next from 10.0.7 to 10.2.3 bgeveritt-snyk/next.js#21

Open

snyk-bot mentioned this pull request Oct 19, 2021

[Snyk] Upgrade next from 10.0.8 to 10.2.3 orysmudi/next.js#2

Open

vercel locked as resolved and limited conversation to collaborators Jan 28, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: replace usage of `fromEntries` in browser bundled `resolve-rewrites.ts` #25208

fix: replace usage of `fromEntries` in browser bundled `resolve-rewrites.ts` #25208

jamsinclair commented May 18, 2021 •

edited

Loading

This comment has been minimized.

timneutkens left a comment

jamsinclair commented May 18, 2021

timneutkens commented May 18, 2021

This comment has been minimized.

ijjk commented May 18, 2021

fix: replace usage of fromEntries in browser bundled resolve-rewrites.ts #25208

fix: replace usage of fromEntries in browser bundled resolve-rewrites.ts #25208

Conversation

jamsinclair commented May 18, 2021 • edited Loading

Bug

Documentation / Examples

Solution

This comment has been minimized.

timneutkens left a comment

Choose a reason for hiding this comment

jamsinclair commented May 18, 2021

timneutkens commented May 18, 2021

This comment has been minimized.

ijjk commented May 18, 2021

Stats from current PR

fix: replace usage of `fromEntries` in browser bundled `resolve-rewrites.ts` #25208

fix: replace usage of `fromEntries` in browser bundled `resolve-rewrites.ts` #25208

jamsinclair commented May 18, 2021 •

edited

Loading