Remove outdated comment about string children being deprecated in `next/link` #30606

Andarist · 2021-10-29T08:13:18Z

I've noticed a comment in the code that this usage is deprecated but I have not seen any notice about this in the console. The comment was referring to the prop-type check but there was no such check.

I've decided to trace this back and it seems that it was removed in this PR. It looks like removing it could be accidental because the PR doesn't mention anyhow that this was made on purpose and the comment mentioning deprecation was not removed.

If this is no longer deprecated then I can change this PR to only remove the comment that mentions this deprecation.

Andarist · 2021-10-29T08:14:03Z

packages/next/client/link.tsx

  if (typeof children === 'string') {
+    console.warn(


I took this message from the moment before it has been removed:

next.js/packages/next/client/link.tsx

Lines 265 to 269 in b0d2a80

if (typeof value === 'string') {

warn(

`Warning: You're using a string directly inside <Link>. This usage has been deprecated. Please add an <a> tag as child of <Link>`

)

}

Andarist · 2021-10-29T08:18:33Z

packages/next/client/link.tsx

@@ -223,8 +223,10 @@ function Link(props: React.PropsWithChildren<LinkProps>) {

  let { children, replace, shallow, scroll, locale } = props

-  // Deprecated. Warning shown by propType check. If the children provided is a string (<Link>example</Link>) we wrap it in an <a> tag


This deprecation was in since 2017 😱 originally introduced here

Andarist · 2021-10-29T15:15:21Z

@ijjk I forgot to wrap this in the env check, fixed now. It shouldn't have any affect on the production bundles

ijjk · 2021-10-29T17:15:54Z

Failing test suites

Commit: 7b69d0d

test/integration/create-next-app/index.test.js

create next app > empty directory
create next app > should use npm as the package manager on supplying --use-npm

Expand output

● create next app › empty directory

thrown: "Exceeded timeout of 90000 ms for a test.
Use jest.setTimeout(newTimeout) to increase the timeout value, if this is a long-running test."

  39 |   // stdin is piped instead of inherited on windows
  40 |   if (process.platform !== 'win32') {
> 41 |     it('empty directory', async () => {
     |     ^
  42 |       await usingTempDir(async (cwd) => {
  43 |         const projectName = 'empty-directory'
  44 |         const res = await run([projectName], { cwd })

  at integration/create-next-app/index.test.js:41:5
  at Object.<anonymous> (integration/create-next-app/index.test.js:24:1)

● create next app › should use npm as the package manager on supplying --use-npm

thrown: "Exceeded timeout of 90000 ms for a test.
Use jest.setTimeout(newTimeout) to increase the timeout value, if this is a long-running test."

  349 |   })
  350 |
> 351 |   it('should use npm as the package manager on supplying --use-npm', async () => {
      |   ^
  352 |     await usingTempDir(async (cwd) => {
  353 |       const projectName = 'use-npm'
  354 |       const res = await run([projectName, '--use-npm'], { cwd })

  at integration/create-next-app/index.test.js:351:3
  at Object.<anonymous> (integration/create-next-app/index.test.js:24:1)

ijjk

Hi, yeah this was removed on purpose so removing the outdated comment would be good 👍

…a children for `next/link`

Andarist · 2021-11-19T08:51:53Z

@ijjk ok, i've repurposed this PR to just remove this outdated comment

ijjk · 2021-11-19T09:07:22Z

Stats from current PR

Default Build (Decrease detected ✓)

General Overall decrease ✓

	vercel/next.js canary	Andarist/next.js bring-back-link-deprecation-warning	Change
buildDuration	18.2s	18.1s	-82ms
buildDurationCached	3.7s	3.7s	⚠️ +50ms
nodeModulesSize	339 MB	339 MB	-149 B

Page Load Tests Overall decrease ⚠️

	vercel/next.js canary	Andarist/next.js bring-back-link-deprecation-warning	Change
/ failed reqs	0	0	✓
/ total time (seconds)	2.95	2.981	⚠️ +0.03
/ avg req/sec	847.55	838.69	⚠️ -8.86
/error-in-render failed reqs	0	0	✓
/error-in-render total time (seconds)	1.363	1.419	⚠️ +0.06
/error-in-render avg req/sec	1834.56	1762.21	⚠️ -72.35

Client Bundles (main, webpack, commons)

	vercel/next.js canary	Andarist/next.js bring-back-link-deprecation-warning	Change
450.HASH.js gzip	179 B	179 B	✓
framework-HASH.js gzip	42.2 kB	42.2 kB	✓
main-HASH.js gzip	28.3 kB	28.3 kB	✓
webpack-HASH.js gzip	1.45 kB	1.45 kB	✓
Overall change	72.2 kB	72.2 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	Andarist/next.js bring-back-link-deprecation-warning	Change
polyfills-HASH.js gzip	31 kB	31 kB	✓
Overall change	31 kB	31 kB	✓

Client Pages

	vercel/next.js canary	Andarist/next.js bring-back-link-deprecation-warning	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	327 B	327 B	✓
dynamic-HASH.js gzip	2.38 kB	2.38 kB	✓
head-HASH.js gzip	350 B	350 B	✓
hooks-HASH.js gzip	635 B	635 B	✓
image-HASH.js gzip	4.45 kB	4.45 kB	✓
index-HASH.js gzip	263 B	263 B	✓
link-HASH.js gzip	1.87 kB	1.87 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	13.3 kB	13.3 kB	✓

Client Build Manifests

	vercel/next.js canary	Andarist/next.js bring-back-link-deprecation-warning	Change
_buildManifest.js gzip	460 B	460 B	✓
Overall change	460 B	460 B	✓

Rendered Page Sizes

	vercel/next.js canary	Andarist/next.js bring-back-link-deprecation-warning	Change
index.html gzip	532 B	532 B	✓
link.html gzip	545 B	545 B	✓
withRouter.html gzip	526 B	526 B	✓
Overall change	1.6 kB	1.6 kB	✓

Diffs

Diff for _buildManifest.js

@@ -13,7 +13,7 @@ self.__BUILD_MANIFEST = {
   "/head": ["static\u002Fchunks\u002Fpages\u002Fhead-7100d3b2a548f0e4.js"],
   "/hooks": ["static\u002Fchunks\u002Fpages\u002Fhooks-ff60bc61acd2698b.js"],
   "/image": ["static\u002Fchunks\u002Fpages\u002Fimage-91f7c5ff8695376c.js"],
-  "/link": ["static\u002Fchunks\u002Fpages\u002Flink-fa969765a079642e.js"],
+  "/link": ["static\u002Fchunks\u002Fpages\u002Flink-678af336607f5345.js"],
   "/routerDirect": [
     "static\u002Fchunks\u002Fpages\u002FrouterDirect-76232dd6bc335a24.js"
   ],

Diff for link-HASH.js

@@ -184,7 +184,6 @@
           shallow = props.shallow,
           scroll = props.scroll,
           locale = props.locale;
-        // Deprecated. Warning shown by propType check. If the children provided is a string (<Link>example</Link>) we wrap it in an <a> tag
         if (typeof children === "string") {
           children = /*#__PURE__*/ _react.default.createElement(
             "a",

Diff for link.html

@@ -27,7 +27,7 @@
       defer=""
     ></script>
     <script
-      src="/_next/static/chunks/pages/link-fa969765a079642e.js"
+      src="/_next/static/chunks/pages/link-678af336607f5345.js"
       defer=""
     ></script>
     <script src="/_next/static/BUILD_ID/_buildManifest.js" defer=""></script>

Default Build with SWC (Decrease detected ✓)

General Overall decrease ✓

	vercel/next.js canary	Andarist/next.js bring-back-link-deprecation-warning	Change
buildDuration	19.9s	19.7s	-267ms
buildDurationCached	3.6s	3.6s	⚠️ +1ms
nodeModulesSize	339 MB	339 MB	-149 B

Page Load Tests Overall decrease ⚠️

	vercel/next.js canary	Andarist/next.js bring-back-link-deprecation-warning	Change
/ failed reqs	0	0	✓
/ total time (seconds)	2.991	2.958	-0.03
/ avg req/sec	835.87	845.16	+9.29
/error-in-render failed reqs	0	0	✓
/error-in-render total time (seconds)	1.398	1.416	⚠️ +0.02
/error-in-render avg req/sec	1788.11	1765.55	⚠️ -22.56

Client Bundles (main, webpack, commons)

	vercel/next.js canary	Andarist/next.js bring-back-link-deprecation-warning	Change
450.HASH.js gzip	179 B	179 B	✓
framework-HASH.js gzip	42.3 kB	42.3 kB	✓
main-HASH.js gzip	28.6 kB	28.6 kB	✓
webpack-HASH.js gzip	1.44 kB	1.44 kB	✓
Overall change	72.5 kB	72.5 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	Andarist/next.js bring-back-link-deprecation-warning	Change
polyfills-HASH.js gzip	31 kB	31 kB	✓
Overall change	31 kB	31 kB	✓

Client Pages

	vercel/next.js canary	Andarist/next.js bring-back-link-deprecation-warning	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.38 kB	2.38 kB	✓
head-HASH.js gzip	342 B	342 B	✓
hooks-HASH.js gzip	622 B	622 B	✓
image-HASH.js gzip	4.47 kB	4.47 kB	✓
index-HASH.js gzip	256 B	256 B	✓
link-HASH.js gzip	1.91 kB	1.91 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	13.2 kB	13.2 kB	✓

Client Build Manifests

	vercel/next.js canary	Andarist/next.js bring-back-link-deprecation-warning	Change
_buildManifest.js gzip	459 B	459 B	✓
Overall change	459 B	459 B	✓

Rendered Page Sizes

	vercel/next.js canary	Andarist/next.js bring-back-link-deprecation-warning	Change
index.html gzip	532 B	532 B	✓
link.html gzip	545 B	545 B	✓
withRouter.html gzip	526 B	526 B	✓
Overall change	1.6 kB	1.6 kB	✓

Diffs

Diff for _buildManifest.js

@@ -13,7 +13,7 @@ self.__BUILD_MANIFEST = {
   "/head": ["static\u002Fchunks\u002Fpages\u002Fhead-7100d3b2a548f0e4.js"],
   "/hooks": ["static\u002Fchunks\u002Fpages\u002Fhooks-ff60bc61acd2698b.js"],
   "/image": ["static\u002Fchunks\u002Fpages\u002Fimage-91f7c5ff8695376c.js"],
-  "/link": ["static\u002Fchunks\u002Fpages\u002Flink-fa969765a079642e.js"],
+  "/link": ["static\u002Fchunks\u002Fpages\u002Flink-678af336607f5345.js"],
   "/routerDirect": [
     "static\u002Fchunks\u002Fpages\u002FrouterDirect-76232dd6bc335a24.js"
   ],

Diff for link-HASH.js

@@ -184,7 +184,6 @@
           shallow = props.shallow,
           scroll = props.scroll,
           locale = props.locale;
-        // Deprecated. Warning shown by propType check. If the children provided is a string (<Link>example</Link>) we wrap it in an <a> tag
         if (typeof children === "string") {
           children = /*#__PURE__*/ _react.default.createElement(
             "a",

Diff for link.html

@@ -27,7 +27,7 @@
       defer=""
     ></script>
     <script
-      src="/_next/static/chunks/pages/link-fa969765a079642e.js"
+      src="/_next/static/chunks/pages/link-678af336607f5345.js"
       defer=""
     ></script>
     <script src="/_next/static/BUILD_ID/_buildManifest.js" defer=""></script>

Commit: d8c1573

ijjk

Thanks for the PR!

Andarist requested review from huozhi, ijjk, padmaia, shuding, styfle and timneutkens as code owners October 29, 2021 08:13

ijjk added the type: next label Oct 29, 2021

Andarist commented Oct 29, 2021

View reviewed changes

This comment has been minimized.

Sign in to view

Andarist force-pushed the bring-back-link-deprecation-warning branch from be29068 to 3bb7342 Compare October 29, 2021 15:15

This comment has been minimized.

Sign in to view

Andarist force-pushed the bring-back-link-deprecation-warning branch from 3bb7342 to 7b69d0d Compare October 29, 2021 16:40

This comment has been minimized.

Sign in to view

ijjk requested changes Nov 19, 2021

View reviewed changes

Andarist added 3 commits November 19, 2021 09:49

Bring back accidentally lost deprecation warning for using string as …

219d4e4

…a children for `next/link`

Wrap warning with the env check

1110136

Remove warning, just go with removed comment about deprecation

d8c1573

Andarist force-pushed the bring-back-link-deprecation-warning branch from 7b69d0d to d8c1573 Compare November 19, 2021 08:50

Andarist changed the title ~~Bring back accidentally lost deprecation warning for using string as a children for next/link~~ Remove outdated comment about string children being deprecated in next/link Nov 19, 2021

ijjk approved these changes Nov 19, 2021

View reviewed changes

Merge branch 'canary' into bring-back-link-deprecation-warning

9d8e10e

ijjk merged commit 9ba31bb into vercel:canary Nov 19, 2021

Andarist deleted the bring-back-link-deprecation-warning branch December 1, 2021 13:30

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove outdated comment about string children being deprecated in `next/link` #30606

Remove outdated comment about string children being deprecated in `next/link` #30606

Andarist commented Oct 29, 2021

Andarist Oct 29, 2021

Andarist Oct 29, 2021

This comment has been minimized.

Andarist commented Oct 29, 2021

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

ijjk commented Oct 29, 2021

ijjk left a comment

Andarist commented Nov 19, 2021

ijjk commented Nov 19, 2021

Diffs

Diffs

ijjk left a comment

	if (typeof value === 'string') {
	warn(
	`Warning: You're using a string directly inside <Link>. This usage has been deprecated. Please add an <a> tag as child of <Link>`
	)
	}

		@@ -223,8 +223,10 @@ function Link(props: React.PropsWithChildren<LinkProps>) {

		let { children, replace, shallow, scroll, locale } = props

		// Deprecated. Warning shown by propType check. If the children provided is a string (<Link>example</Link>) we wrap it in an <a> tag

Remove outdated comment about string children being deprecated in next/link #30606

Remove outdated comment about string children being deprecated in next/link #30606

Conversation

Andarist commented Oct 29, 2021

Andarist Oct 29, 2021

Choose a reason for hiding this comment

Andarist Oct 29, 2021

Choose a reason for hiding this comment

This comment has been minimized.

Andarist commented Oct 29, 2021

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

ijjk commented Oct 29, 2021

Failing test suites

ijjk left a comment

Choose a reason for hiding this comment

Andarist commented Nov 19, 2021

ijjk commented Nov 19, 2021

Stats from current PR

Diffs

Diffs

ijjk left a comment

Choose a reason for hiding this comment

Remove outdated comment about string children being deprecated in `next/link` #30606

Remove outdated comment about string children being deprecated in `next/link` #30606