Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Ensure dev overlay uses basePath for requests #14475

Merged
merged 4 commits into from
Jun 23, 2020
Merged

Ensure dev overlay uses basePath for requests #14475

merged 4 commits into from
Jun 23, 2020

Conversation

ijjk
Copy link
Member

@ijjk ijjk commented Jun 23, 2020

This fixes the basePath not being used when making requests for the react-dev-overlay. We might want to pass down the basePath to the overlay instead of relying on the process.env injection although may be fine this way, will defer to @Timer for preferred way to pass this value to the dev-overlay

Closes: #14470

@ijjk
Copy link
Member Author

ijjk commented Jun 23, 2020

Stats from current PR

Default Server Mode (Increase detected ⚠️)
General
vercel/next.js canary ijjk/next.js fix/basepath-dev-overlay Change
buildDuration 11.7s 11.8s ⚠️ +128ms
nodeModulesSize 67.1 MB 67.1 MB ✓
Page Load Tests Overall increase ✓
vercel/next.js canary ijjk/next.js fix/basepath-dev-overlay Change
/ failed reqs 0 0 ✓
/ total time (seconds) 1.996 1.998 0
/ avg req/sec 1252.57 1251.12 -1.45
/error-in-render failed reqs 0 0 ✓
/error-in-render total time (seconds) 1.178 1.162 -0.02
/error-in-render avg req/sec 2121.81 2151.9 ⚠️ +30.09
Client Bundles (main, webpack, commons)
vercel/next.js canary ijjk/next.js fix/basepath-dev-overlay Change
main-HASH.js gzip 6.52 kB 6.52 kB ✓
webpack-HASH.js gzip 746 B 746 B ✓
19b7e98f51cc..62e5.js gzip 10.6 kB 10.6 kB ✓
framework.HASH.js gzip 39.1 kB 39.1 kB ✓
Overall change 57 kB 57 kB ✓
Client Bundles (main, webpack, commons) Modern
vercel/next.js canary ijjk/next.js fix/basepath-dev-overlay Change
main-HASH.module.js gzip 5.61 kB 5.61 kB ✓
webpack-HASH..dule.js gzip 746 B 746 B ✓
19b7e98f51cc..dule.js gzip 6.93 kB 6.93 kB ✓
framework.HA..dule.js gzip 39.1 kB 39.1 kB ✓
Overall change 52.4 kB 52.4 kB ✓
Legacy Client Bundles (polyfills)
vercel/next.js canary ijjk/next.js fix/basepath-dev-overlay Change
polyfills-HASH.js gzip 26.3 kB 26.3 kB ✓
Overall change 26.3 kB 26.3 kB ✓
Client Build Manifests
vercel/next.js canary ijjk/next.js fix/basepath-dev-overlay Change
_buildManifest.js gzip 267 B 267 B ✓
_buildManife..dule.js gzip 272 B 272 B ✓
Overall change 539 B 539 B ✓
Rendered Page Sizes
vercel/next.js canary ijjk/next.js fix/basepath-dev-overlay Change
index.html gzip 956 B 956 B ✓
link.html gzip 961 B 961 B ✓
withRouter.html gzip 948 B 948 B ✓
Overall change 2.87 kB 2.87 kB ✓
Serverless Mode
General
vercel/next.js canary ijjk/next.js fix/basepath-dev-overlay Change
buildDuration 13.1s 12.8s -373ms
nodeModulesSize 67.1 MB 67.1 MB ✓
Client Bundles (main, webpack, commons)
vercel/next.js canary ijjk/next.js fix/basepath-dev-overlay Change
main-HASH.js gzip 6.52 kB 6.52 kB ✓
webpack-HASH.js gzip 746 B 746 B ✓
19b7e98f51cc..62e5.js gzip 10.6 kB 10.6 kB ✓
framework.HASH.js gzip 39.1 kB 39.1 kB ✓
Overall change 57 kB 57 kB ✓
Client Bundles (main, webpack, commons) Modern
vercel/next.js canary ijjk/next.js fix/basepath-dev-overlay Change
main-HASH.module.js gzip 5.61 kB 5.61 kB ✓
webpack-HASH..dule.js gzip 746 B 746 B ✓
19b7e98f51cc..dule.js gzip 6.93 kB 6.93 kB ✓
framework.HA..dule.js gzip 39.1 kB 39.1 kB ✓
Overall change 52.4 kB 52.4 kB ✓
Legacy Client Bundles (polyfills)
vercel/next.js canary ijjk/next.js fix/basepath-dev-overlay Change
polyfills-HASH.js gzip 26.3 kB 26.3 kB ✓
Overall change 26.3 kB 26.3 kB ✓
Client Build Manifests
vercel/next.js canary ijjk/next.js fix/basepath-dev-overlay Change
_buildManifest.js gzip 267 B 267 B ✓
_buildManife..dule.js gzip 272 B 272 B ✓
Overall change 539 B 539 B ✓
Serverless bundles
vercel/next.js canary ijjk/next.js fix/basepath-dev-overlay Change
_error.js 875 kB 875 kB ✓
404.html 4.17 kB 4.17 kB ✓
hooks.html 3.79 kB 3.79 kB ✓
index.js 875 kB 875 kB ✓
link.js 914 kB 914 kB ✓
routerDirect.js 906 kB 906 kB ✓
withRouter.js 906 kB 906 kB ✓
Overall change 4.48 MB 4.48 MB ✓
Commit: b3cf533

@ijjk ijjk requested a review from Timer June 23, 2020 02:48
@ijjk
Copy link
Member Author

ijjk commented Jun 23, 2020

Stats from current PR

Default Server Mode (Decrease detected ✓)
General
vercel/next.js canary ijjk/next.js fix/basepath-dev-overlay Change
buildDuration 11.1s 11.2s ⚠️ +88ms
nodeModulesSize 67.1 MB 67.1 MB ✓
Page Load Tests Overall decrease ⚠️
vercel/next.js canary ijjk/next.js fix/basepath-dev-overlay Change
/ failed reqs 0 0 ✓
/ total time (seconds) 1.946 1.854 -0.09
/ avg req/sec 1284.91 1348.38 ⚠️ +63.47
/error-in-render failed reqs 0 0 ✓
/error-in-render total time (seconds) 1.104 1.148 ⚠️ +0.04
/error-in-render avg req/sec 2265.48 2177.17 -88.31
Client Bundles (main, webpack, commons)
vercel/next.js canary ijjk/next.js fix/basepath-dev-overlay Change
main-HASH.js gzip 6.52 kB 6.52 kB ✓
webpack-HASH.js gzip 746 B 746 B ✓
19b7e98f51cc..62e5.js gzip 10.6 kB 10.6 kB ✓
framework.HASH.js gzip 39.1 kB 39.1 kB ✓
Overall change 57 kB 57 kB ✓
Client Bundles (main, webpack, commons) Modern
vercel/next.js canary ijjk/next.js fix/basepath-dev-overlay Change
main-HASH.module.js gzip 5.61 kB 5.61 kB ✓
webpack-HASH..dule.js gzip 746 B 746 B ✓
19b7e98f51cc..dule.js gzip 6.93 kB 6.93 kB ✓
framework.HA..dule.js gzip 39.1 kB 39.1 kB ✓
Overall change 52.4 kB 52.4 kB ✓
Legacy Client Bundles (polyfills)
vercel/next.js canary ijjk/next.js fix/basepath-dev-overlay Change
polyfills-HASH.js gzip 26.3 kB 26.3 kB ✓
Overall change 26.3 kB 26.3 kB ✓
Client Build Manifests
vercel/next.js canary ijjk/next.js fix/basepath-dev-overlay Change
_buildManifest.js gzip 267 B 267 B ✓
_buildManife..dule.js gzip 272 B 272 B ✓
Overall change 539 B 539 B ✓
Rendered Page Sizes
vercel/next.js canary ijjk/next.js fix/basepath-dev-overlay Change
index.html gzip 956 B 956 B ✓
link.html gzip 961 B 961 B ✓
withRouter.html gzip 948 B 948 B ✓
Overall change 2.87 kB 2.87 kB ✓
Serverless Mode
General
vercel/next.js canary ijjk/next.js fix/basepath-dev-overlay Change
buildDuration 12.1s 11.7s -382ms
nodeModulesSize 67.1 MB 67.1 MB ✓
Client Bundles (main, webpack, commons)
vercel/next.js canary ijjk/next.js fix/basepath-dev-overlay Change
main-HASH.js gzip 6.52 kB 6.52 kB ✓
webpack-HASH.js gzip 746 B 746 B ✓
19b7e98f51cc..62e5.js gzip 10.6 kB 10.6 kB ✓
framework.HASH.js gzip 39.1 kB 39.1 kB ✓
Overall change 57 kB 57 kB ✓
Client Bundles (main, webpack, commons) Modern
vercel/next.js canary ijjk/next.js fix/basepath-dev-overlay Change
main-HASH.module.js gzip 5.61 kB 5.61 kB ✓
webpack-HASH..dule.js gzip 746 B 746 B ✓
19b7e98f51cc..dule.js gzip 6.93 kB 6.93 kB ✓
framework.HA..dule.js gzip 39.1 kB 39.1 kB ✓
Overall change 52.4 kB 52.4 kB ✓
Legacy Client Bundles (polyfills)
vercel/next.js canary ijjk/next.js fix/basepath-dev-overlay Change
polyfills-HASH.js gzip 26.3 kB 26.3 kB ✓
Overall change 26.3 kB 26.3 kB ✓
Client Build Manifests
vercel/next.js canary ijjk/next.js fix/basepath-dev-overlay Change
_buildManifest.js gzip 267 B 267 B ✓
_buildManife..dule.js gzip 272 B 272 B ✓
Overall change 539 B 539 B ✓
Serverless bundles
vercel/next.js canary ijjk/next.js fix/basepath-dev-overlay Change
_error.js 875 kB 875 kB ✓
404.html 4.17 kB 4.17 kB ✓
hooks.html 3.79 kB 3.79 kB ✓
index.js 875 kB 875 kB ✓
link.js 914 kB 914 kB ✓
routerDirect.js 906 kB 906 kB ✓
withRouter.js 906 kB 906 kB ✓
Overall change 4.48 MB 4.48 MB ✓
Commit: 20e16ec

Timer
Timer requested changes Jun 23, 2020
View reviewed changes
Copy link
Member

@Timer Timer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs win32 test in branch
image

timneutkens
timneutkens previously approved these changes Jun 23, 2020
View reviewed changes
@ijjk ijjk requested review from Timer and timneutkens June 23, 2020 14:57
@ijjk
Copy link
Member Author

ijjk commented Jun 23, 2020

Stats from current PR

Default Server Mode (Decrease detected ✓)
General
vercel/next.js canary ijjk/next.js fix/basepath-dev-overlay Change
buildDuration 14.2s 15.2s ⚠️ +1.1s
nodeModulesSize 67.1 MB 67.1 MB ✓
Page Load Tests Overall decrease ⚠️
vercel/next.js canary ijjk/next.js fix/basepath-dev-overlay Change
/ failed reqs 0 0 ✓
/ total time (seconds) 2.465 2.558 ⚠️ +0.09
/ avg req/sec 1014.15 977.38 -36.77
/error-in-render failed reqs 0 0 ✓
/error-in-render total time (seconds) 1.731 1.843 ⚠️ +0.11
/error-in-render avg req/sec 1444.3 1356.24 -88.06
Client Bundles (main, webpack, commons)
vercel/next.js canary ijjk/next.js fix/basepath-dev-overlay Change
main-HASH.js gzip 6.49 kB 6.49 kB ✓
webpack-HASH.js gzip 746 B 746 B ✓
19b7e98f51cc..14a7.js gzip 10.7 kB 10.7 kB ✓
framework.HASH.js gzip 39.1 kB 39.1 kB ✓
Overall change 57.1 kB 57.1 kB ✓
Client Bundles (main, webpack, commons) Modern
vercel/next.js canary ijjk/next.js fix/basepath-dev-overlay Change
main-HASH.module.js gzip 5.57 kB 5.57 kB ✓
webpack-HASH..dule.js gzip 746 B 746 B ✓
19b7e98f51cc..dule.js gzip 7.07 kB 7.07 kB ✓
framework.HA..dule.js gzip 39.1 kB 39.1 kB ✓
Overall change 52.5 kB 52.5 kB ✓
Legacy Client Bundles (polyfills)
vercel/next.js canary ijjk/next.js fix/basepath-dev-overlay Change
polyfills-HASH.js gzip 26.3 kB 26.3 kB ✓
Overall change 26.3 kB 26.3 kB ✓
Client Build Manifests
vercel/next.js canary ijjk/next.js fix/basepath-dev-overlay Change
_buildManifest.js gzip 266 B 266 B ✓
_buildManife..dule.js gzip 272 B 272 B ✓
Overall change 538 B 538 B ✓
Rendered Page Sizes
vercel/next.js canary ijjk/next.js fix/basepath-dev-overlay Change
index.html gzip 955 B 955 B ✓
link.html gzip 962 B 962 B ✓
withRouter.html gzip 947 B 947 B ✓
Overall change 2.86 kB 2.86 kB ✓
Serverless Mode
General
vercel/next.js canary ijjk/next.js fix/basepath-dev-overlay Change
buildDuration 15s 15.4s ⚠️ +446ms
nodeModulesSize 67.1 MB 67.1 MB ✓
Client Bundles (main, webpack, commons)
vercel/next.js canary ijjk/next.js fix/basepath-dev-overlay Change
main-HASH.js gzip 6.49 kB 6.49 kB ✓
webpack-HASH.js gzip 746 B 746 B ✓
19b7e98f51cc..14a7.js gzip 10.7 kB 10.7 kB ✓
framework.HASH.js gzip 39.1 kB 39.1 kB ✓
Overall change 57.1 kB 57.1 kB ✓
Client Bundles (main, webpack, commons) Modern
vercel/next.js canary ijjk/next.js fix/basepath-dev-overlay Change
main-HASH.module.js gzip 5.57 kB 5.57 kB ✓
webpack-HASH..dule.js gzip 746 B 746 B ✓
19b7e98f51cc..dule.js gzip 7.07 kB 7.07 kB ✓
framework.HA..dule.js gzip 39.1 kB 39.1 kB ✓
Overall change 52.5 kB 52.5 kB ✓
Legacy Client Bundles (polyfills)
vercel/next.js canary ijjk/next.js fix/basepath-dev-overlay Change
polyfills-HASH.js gzip 26.3 kB 26.3 kB ✓
Overall change 26.3 kB 26.3 kB ✓
Client Build Manifests
vercel/next.js canary ijjk/next.js fix/basepath-dev-overlay Change
_buildManifest.js gzip 266 B 266 B ✓
_buildManife..dule.js gzip 272 B 272 B ✓
Overall change 538 B 538 B ✓
Serverless bundles
vercel/next.js canary ijjk/next.js fix/basepath-dev-overlay Change
_error.js 875 kB 875 kB ✓
404.html 4.17 kB 4.17 kB ✓
hooks.html 3.79 kB 3.79 kB ✓
index.js 875 kB 875 kB ✓
link.js 916 kB 916 kB ✓
routerDirect.js 908 kB 908 kB ✓
withRouter.js 908 kB 908 kB ✓
Overall change 4.49 MB 4.49 MB ✓
Commit: 843473e

@kodiakhq kodiakhq bot merged commit 2136c46 into vercel:canary Jun 23, 2020
@Timer Timer deleted the fix/basepath-dev-overlay branch June 23, 2020 15:14
rokinsky pushed a commit to rokinsky/next.js that referenced this pull request Jul 11, 2020
@ijjk @rokinsky
Ensure dev overlay uses basePath for requests (vercel#14475)
7f726dc 
This fixes the `basePath` not being used when making requests for the `react-dev-overlay`. We might want to pass down the `basePath` to the overlay instead of relying on the `process.env` injection although may be fine this way, will defer to @Timer for preferred way to pass this value to the dev-overlay

Closes: vercel#14470
@vercel vercel locked as resolved and limited conversation to collaborators Jan 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Timer Timer Timer approved these changes

@lfades lfades Awaiting requested review from lfades

@timneutkens timneutkens Awaiting requested review from timneutkens

Assignees
No one assigned
Labels
type: next
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

react-dev-overlay does not work correctly with basePath
3 participants
@ijjk @Timer @timneutkens