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.

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

I18n context initial props #21930

Merged
merged 19 commits into from
May 10, 2021
Merged

Conversation

Gigiz
Copy link
Contributor

@Gigiz Gigiz commented Feb 6, 2021

Add i18n locale props on getInitialProps context.

In order to manage custom errors, such as a 410 status code, and to guarantee the internationalization of the contents, it is necessary have access to the i18n properties in getInitialProps context.

Discussion: #18396

@ijjk

This comment has been minimized.

@BjoernRave
Copy link

@aralroca

@ijjk

This comment has been minimized.

Copy link
Member

@timneutkens timneutkens left a comment

Choose a reason for hiding this comment

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

This looks incorrect, I doubt it works for client-side navigation 🤔

@Gigiz
Copy link
Contributor Author

Gigiz commented Feb 11, 2021

It works like a charm. Locale props are correctly valued for initialProps context.

Copy link
Member

@timneutkens timneutkens left a comment

Choose a reason for hiding this comment

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

As said it's very unlikely this works correctly currently given you did not change cases like https://github.com/vercel/next.js/blob/canary/packages/next/next-server/lib/router/router.ts#L1230-L1234 and https://github.com/vercel/next.js/blob/canary/packages/next/next-server/lib/router/router.ts#L1310-L1317

There are no tests for client-side navigation included in the PR, can you add them.

@Gigiz
Copy link
Contributor Author

Gigiz commented Feb 12, 2021

Sorry my bad.
I added props on the client side routing method

@ijjk

This comment has been minimized.

@ijjk

This comment has been minimized.

@ijjk

This comment has been minimized.

@ijjk

This comment has been minimized.

@Gigiz Gigiz requested a review from timneutkens February 15, 2021 08:13
@ijjk

This comment has been minimized.

@ijjk

This comment has been minimized.

@aralroca
Copy link
Contributor

aralroca commented Apr 7, 2021

How is it going? I can't wait to have this merged 😊

@ValentinH
Copy link
Contributor

ValentinH commented Apr 15, 2021

Most likely because you increased the bundle size of Next.js while adding this feature. You'll have to ensure it does not increase bundle size.

@timneutkens I don't see how not to add code to support this feature. Thus, the bundle size of Next has to increase. How do you usually add features without increasing the bundle size?

@satazor
Copy link

satazor commented Apr 18, 2021

The increase is necessary and minimal (400 bytes). I'm hoping this gets merged soon as I need this too.

@Iliyass
Copy link

Iliyass commented May 6, 2021

@aralroca thanks for your work,
@timneutkens please, can you make this possible, we need this PR to be merged!

timneutkens
timneutkens previously approved these changes May 7, 2021
Copy link
Member

@timneutkens timneutkens left a comment

Choose a reason for hiding this comment

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

Looks fine to land now.

@timneutkens
Copy link
Member

Most likely because you increased the bundle size of Next.js while adding this feature. You'll have to ensure it does not increase bundle size.

@timneutkens I don't see how not to add code to support this feature. Thus, the bundle size of Next has to increase. How do you usually add features without increasing the bundle size?

By optimizing other parts of the client bundles.

@kodiakhq kodiakhq bot requested a review from shuding as a code owner May 7, 2021 09:32
@ijjk

This comment has been minimized.

@ijjk
Copy link
Member

ijjk commented May 7, 2021

Failing test suites

Commit: ebd6551

test/integration/build-output/test/index.test.js

  • Build Output > Basic Application Output > should not deviate from snapshot
Expand output

● Build Output › Basic Application Output › should not deviate from snapshot

expect(received).toBeCloseTo(expected, precision)

Expected: 63.5
Received: 63.6

Expected precision:    1
Expected difference: < 0.05
Received difference:   0.10000000000000142

  100 |
  101 |       // should be no bigger than 64.8 kb
> 102 |       expect(parseFloat(indexFirstLoad)).toBeCloseTo(63.5, 1)
      |                                          ^
  103 |       expect(indexFirstLoad.endsWith('kB')).toBe(true)
  104 |
  105 |       expect(parseFloat(err404Size)).toBeCloseTo(3.04, 1)

  at Object.<anonymous> (integration/build-output/test/index.test.js:102:42)

@timneutkens
Copy link
Member

Looks like this is failing against the latest canary, can you update the test @Gigiz. Thanks!

@ijjk
Copy link
Member

ijjk commented May 10, 2021

Stats from current PR

Default Server Mode (Increase detected ⚠️)
General Overall decrease ✓
vercel/next.js canary Gigiz/next.js i18n-context-initial-props Change
buildDuration 14.4s 14.6s ⚠️ +223ms
buildDurationCached 4.6s 4.4s -226ms
nodeModulesSize 46.5 MB 46.4 MB -89.1 kB
Page Load Tests Overall increase ✓
vercel/next.js canary Gigiz/next.js i18n-context-initial-props Change
/ failed reqs 0 0
/ total time (seconds) 2.464 2.443 -0.02
/ avg req/sec 1014.54 1023.15 +8.61
/error-in-render failed reqs 0 0
/error-in-render total time (seconds) 1.458 1.462 0
/error-in-render avg req/sec 1714.96 1710.47 ⚠️ -4.49
Client Bundles (main, webpack, commons) Overall increase ⚠️
vercel/next.js canary Gigiz/next.js i18n-context-initial-props Change
framework-HASH.js gzip 39.3 kB 39.3 kB
main-HASH.js gzip 19.3 kB 19.3 kB ⚠️ +21 B
webpack-HASH.js gzip 996 B 996 B
Overall change 59.6 kB 59.6 kB ⚠️ +21 B
Legacy Client Bundles (polyfills)
vercel/next.js canary Gigiz/next.js i18n-context-initial-props 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 Gigiz/next.js i18n-context-initial-props Change
_app-HASH.js gzip 1.02 kB 1.02 kB
_error-HASH.js gzip 3.05 kB 3.05 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
99a142a5cfae..804.css gzip 125 B 125 B
Overall change 8.52 kB 8.52 kB
Client Build Manifests
vercel/next.js canary Gigiz/next.js i18n-context-initial-props Change
_buildManifest.js gzip 394 B 394 B
Overall change 394 B 394 B
Rendered Page Sizes Overall decrease ✓
vercel/next.js canary Gigiz/next.js i18n-context-initial-props Change
index.html gzip 560 B 561 B ⚠️ +1 B
link.html gzip 569 B 568 B -1 B
withRouter.html gzip 557 B 556 B -1 B
Overall change 1.69 kB 1.69 kB -1 B

Diffs

Diff for main-HASH.js
@@ -4690,7 +4690,10 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
                                     {
                                       pathname: pathname,
                                       query: query,
-                                      asPath: as
+                                      asPath: as,
+                                      locale: _this2.locale,
+                                      locales: _this2.locales,
+                                      defaultLocale: _this2.defaultLocale
                                     }
                                   );
                             });
Diff for index.html
@@ -7,7 +7,7 @@
     <noscript data-n-css=""></noscript>
     <link
       rel="preload"
-      href="/_next/static/chunks/webpack-d1bbd0644f0e2216539c.js"
+      href="/_next/static/chunks/webpack-6b8ad465fa3269ad2efe.js"
       as="script"
     />
     <link
@@ -17,7 +17,7 @@
     />
     <link
       rel="preload"
-      href="/_next/static/chunks/main-86d84adc5905b9d00dd5.js"
+      href="/_next/static/chunks/main-679dd30ce6e9c7c35943.js"
       as="script"
     />
     <link
@@ -48,7 +48,7 @@
       src="/_next/static/chunks/polyfills-01e9caf46d9a7f038c09.js"
     ></script>
     <script
-      src="/_next/static/chunks/webpack-d1bbd0644f0e2216539c.js"
+      src="/_next/static/chunks/webpack-6b8ad465fa3269ad2efe.js"
       async=""
     ></script>
     <script
@@ -56,7 +56,7 @@
       async=""
     ></script>
     <script
-      src="/_next/static/chunks/main-86d84adc5905b9d00dd5.js"
+      src="/_next/static/chunks/main-679dd30ce6e9c7c35943.js"
       async=""
     ></script>
     <script
Diff for link.html
@@ -7,7 +7,7 @@
     <noscript data-n-css=""></noscript>
     <link
       rel="preload"
-      href="/_next/static/chunks/webpack-d1bbd0644f0e2216539c.js"
+      href="/_next/static/chunks/webpack-6b8ad465fa3269ad2efe.js"
       as="script"
     />
     <link
@@ -17,7 +17,7 @@
     />
     <link
       rel="preload"
-      href="/_next/static/chunks/main-86d84adc5905b9d00dd5.js"
+      href="/_next/static/chunks/main-679dd30ce6e9c7c35943.js"
       as="script"
     />
     <link
@@ -53,7 +53,7 @@
       src="/_next/static/chunks/polyfills-01e9caf46d9a7f038c09.js"
     ></script>
     <script
-      src="/_next/static/chunks/webpack-d1bbd0644f0e2216539c.js"
+      src="/_next/static/chunks/webpack-6b8ad465fa3269ad2efe.js"
       async=""
     ></script>
     <script
@@ -61,7 +61,7 @@
       async=""
     ></script>
     <script
-      src="/_next/static/chunks/main-86d84adc5905b9d00dd5.js"
+      src="/_next/static/chunks/main-679dd30ce6e9c7c35943.js"
       async=""
     ></script>
     <script
Diff for withRouter.html
@@ -7,7 +7,7 @@
     <noscript data-n-css=""></noscript>
     <link
       rel="preload"
-      href="/_next/static/chunks/webpack-d1bbd0644f0e2216539c.js"
+      href="/_next/static/chunks/webpack-6b8ad465fa3269ad2efe.js"
       as="script"
     />
     <link
@@ -17,7 +17,7 @@
     />
     <link
       rel="preload"
-      href="/_next/static/chunks/main-86d84adc5905b9d00dd5.js"
+      href="/_next/static/chunks/main-679dd30ce6e9c7c35943.js"
       as="script"
     />
     <link
@@ -48,7 +48,7 @@
       src="/_next/static/chunks/polyfills-01e9caf46d9a7f038c09.js"
     ></script>
     <script
-      src="/_next/static/chunks/webpack-d1bbd0644f0e2216539c.js"
+      src="/_next/static/chunks/webpack-6b8ad465fa3269ad2efe.js"
       async=""
     ></script>
     <script
@@ -56,7 +56,7 @@
       async=""
     ></script>
     <script
-      src="/_next/static/chunks/main-86d84adc5905b9d00dd5.js"
+      src="/_next/static/chunks/main-679dd30ce6e9c7c35943.js"
       async=""
     ></script>
     <script

Serverless Mode (Increase detected ⚠️)
General Overall decrease ✓
vercel/next.js canary Gigiz/next.js i18n-context-initial-props Change
buildDuration 18.3s 17.8s -479ms
buildDurationCached 6.5s 6.5s -78ms
nodeModulesSize 46.5 MB 46.4 MB -89.1 kB
Client Bundles (main, webpack, commons) Overall increase ⚠️
vercel/next.js canary Gigiz/next.js i18n-context-initial-props Change
framework-HASH.js gzip 39.3 kB 39.3 kB
main-HASH.js gzip 19.3 kB 19.3 kB ⚠️ +21 B
webpack-HASH.js gzip 996 B 996 B
Overall change 59.6 kB 59.6 kB ⚠️ +21 B
Legacy Client Bundles (polyfills)
vercel/next.js canary Gigiz/next.js i18n-context-initial-props 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 Gigiz/next.js i18n-context-initial-props Change
_app-HASH.js gzip 1.02 kB 1.02 kB
_error-HASH.js gzip 3.05 kB 3.05 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
99a142a5cfae..804.css gzip 125 B 125 B
Overall change 8.52 kB 8.52 kB
Client Build Manifests
vercel/next.js canary Gigiz/next.js i18n-context-initial-props Change
_buildManifest.js gzip 394 B 394 B
Overall change 394 B 394 B
Serverless bundles Overall increase ⚠️
vercel/next.js canary Gigiz/next.js i18n-context-initial-props Change
_error.js 1.34 MB 1.34 MB ⚠️ +91 B
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 1.35 MB 1.35 MB ⚠️ +91 B
link.js 1.4 MB 1.4 MB ⚠️ +194 B
routerDirect.js 1.39 MB 1.4 MB ⚠️ +194 B
withRouter.js 1.39 MB 1.4 MB ⚠️ +194 B
Overall change 6.9 MB 6.9 MB ⚠️ +764 B

Webpack 4 Mode (Increase detected ⚠️)
General Overall decrease ✓
vercel/next.js canary Gigiz/next.js i18n-context-initial-props Change
buildDuration 12.1s 12.1s -42ms
buildDurationCached 5.1s 5.1s -67ms
nodeModulesSize 46.5 MB 46.4 MB -89.1 kB
Page Load Tests Overall decrease ⚠️
vercel/next.js canary Gigiz/next.js i18n-context-initial-props Change
/ failed reqs 0 0
/ total time (seconds) 2.636 2.558 -0.08
/ avg req/sec 948.42 977.5 +29.08
/error-in-render failed reqs 0 0
/error-in-render total time (seconds) 1.539 1.572 ⚠️ +0.03
/error-in-render avg req/sec 1623.99 1589.94 ⚠️ -34.05
Client Bundles (main, webpack, commons) Overall increase ⚠️
vercel/next.js canary Gigiz/next.js i18n-context-initial-props Change
677f882d2ed8..HASH.js gzip 13.3 kB 13.3 kB ⚠️ +14 B
framework.HASH.js gzip 39 kB 39 kB
main-HASH.js gzip 7.19 kB 7.19 kB
webpack-HASH.js gzip 751 B 751 B
Overall change 60.2 kB 60.2 kB ⚠️ +14 B
Legacy Client Bundles (polyfills)
vercel/next.js canary Gigiz/next.js i18n-context-initial-props 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 Gigiz/next.js i18n-context-initial-props Change
_app-HASH.js gzip 1.28 kB 1.28 kB
_error-HASH.js gzip 3.72 kB 3.72 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
21c68fa65a48..217.css gzip 125 B 125 B
Overall change 9.37 kB 9.37 kB
Client Build Manifests
vercel/next.js canary Gigiz/next.js i18n-context-initial-props Change
_buildManifest.js gzip 420 B 420 B
Overall change 420 B 420 B
Rendered Page Sizes Overall decrease ✓
vercel/next.js canary Gigiz/next.js i18n-context-initial-props Change
index.html gzip 613 B 612 B -1 B
link.html gzip 620 B 620 B
withRouter.html gzip 607 B 606 B -1 B
Overall change 1.84 kB 1.84 kB -2 B

Diffs

Diff for 677f882d2ed8..c4df.HASH.js
@@ -2771,7 +2771,10 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
                                     {
                                       pathname: pathname,
                                       query: query,
-                                      asPath: as
+                                      asPath: as,
+                                      locale: _this2.locale,
+                                      locales: _this2.locales,
+                                      defaultLocale: _this2.defaultLocale
                                     }
                                   );
                             });
Diff for index.html
@@ -17,7 +17,7 @@
     />
     <link
       rel="preload"
-      href="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.034e4b1503019c911a82.js"
+      href="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.316105931a71d2e2e072.js"
       as="script"
     />
     <link
@@ -61,7 +61,7 @@
       async=""
     ></script>
     <script
-      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.034e4b1503019c911a82.js"
+      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.316105931a71d2e2e072.js"
       async=""
     ></script>
     <script
Diff for link.html
@@ -17,7 +17,7 @@
     />
     <link
       rel="preload"
-      href="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.034e4b1503019c911a82.js"
+      href="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.316105931a71d2e2e072.js"
       as="script"
     />
     <link
@@ -66,7 +66,7 @@
       async=""
     ></script>
     <script
-      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.034e4b1503019c911a82.js"
+      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.316105931a71d2e2e072.js"
       async=""
     ></script>
     <script
Diff for withRouter.html
@@ -17,7 +17,7 @@
     />
     <link
       rel="preload"
-      href="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.034e4b1503019c911a82.js"
+      href="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.316105931a71d2e2e072.js"
       as="script"
     />
     <link
@@ -61,7 +61,7 @@
       async=""
     ></script>
     <script
-      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.034e4b1503019c911a82.js"
+      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.316105931a71d2e2e072.js"
       async=""
     ></script>
     <script
Commit: 2bd9c9e

@timneutkens timneutkens merged commit 59564af into vercel:canary May 10, 2021
@hedwiggggg
Copy link

hedwiggggg commented May 12, 2021

Seem's, the types are still missing:

export interface NextPageContext {
/**
* Error object if encountered during rendering
*/
err?: (Error & { statusCode?: number }) | null
/**
* `HTTP` request object.
*/
req?: IncomingMessage
/**
* `HTTP` response object.
*/
res?: ServerResponse
/**
* Path section of `URL`.
*/
pathname: string
/**
* Query string section of `URL` parsed as an object.
*/
query: ParsedUrlQuery
/**
* `String` of the actual path including query.
*/
asPath?: string
/**
* `Component` the tree of the App to use if needing to render separately
*/
AppTree: AppTreeType
}

@rokinsky
Copy link
Contributor

Seem's, the types are still missing:

export interface NextPageContext {
/**
* Error object if encountered during rendering
*/
err?: (Error & { statusCode?: number }) | null
/**
* `HTTP` request object.
*/
req?: IncomingMessage
/**
* `HTTP` response object.
*/
res?: ServerResponse
/**
* Path section of `URL`.
*/
pathname: string
/**
* Query string section of `URL` parsed as an object.
*/
query: ParsedUrlQuery
/**
* `String` of the actual path including query.
*/
asPath?: string
/**
* `Component` the tree of the App to use if needing to render separately
*/
AppTree: AppTreeType
}

I've noticed this as well. I hope #25363 will be merged to canary soon.

kodiakhq bot pushed a commit that referenced this pull request May 22, 2021
## Bug

- [x] Related issues linked using `fixes #number`
- [ ] Integration tests added

## 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.

## Documentation / Examples

- [] Make sure the linting passes

Ref: #21930
flybayer pushed a commit to blitz-js/next.js that referenced this pull request Jun 1, 2021
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
flybayer pushed a commit to blitz-js/next.js that referenced this pull request Jun 1, 2021
## Bug

- [x] Related issues linked using `fixes #number`
- [ ] Integration tests added

## 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.

## Documentation / Examples

- [] Make sure the linting passes

Ref: vercel#21930
@vercel vercel locked as resolved and limited conversation to collaborators Jan 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants