From efa32c4caa6854970c071ed9933a783e2528f860 Mon Sep 17 00:00:00 2001 From: eps1lon Date: Wed, 20 Nov 2024 20:05:53 +0100 Subject: [PATCH 1/5] Lint against GCC `optimizeArgumentsArray` passes --- scripts/rollup/validate/eslintrc.cjs.js | 11 +++++++++++ scripts/rollup/validate/eslintrc.cjs2015.js | 11 +++++++++++ scripts/rollup/validate/eslintrc.esm.js | 11 +++++++++++ scripts/rollup/validate/eslintrc.fb.js | 11 +++++++++++ scripts/rollup/validate/eslintrc.rn.js | 11 +++++++++++ 5 files changed, 55 insertions(+) diff --git a/scripts/rollup/validate/eslintrc.cjs.js b/scripts/rollup/validate/eslintrc.cjs.js index c91c298edf1be..d685efdd97b39 100644 --- a/scripts/rollup/validate/eslintrc.cjs.js +++ b/scripts/rollup/validate/eslintrc.cjs.js @@ -86,6 +86,17 @@ module.exports = { rules: { 'no-undef': 'error', 'no-shadow-restricted-names': 'error', + 'no-restricted-syntax': [ + 'error', + // TODO: Can be removed once we upgrade GCC to a version without `optimizeArgumentsArray` optimization. + { + selector: 'Identifier[name=/^JSCompiler_OptimizeArgumentsArray_/]', + message: + 'Google Closure Compiler optimized `arguments` access. ' + + 'This affects function arity. ' + + 'Access `arguments.length` to avoid this optimization', + }, + ], }, // These plugins aren't used, but eslint complains if an eslint-ignore comment diff --git a/scripts/rollup/validate/eslintrc.cjs2015.js b/scripts/rollup/validate/eslintrc.cjs2015.js index 01f387e1d7188..dfa6a9cd07934 100644 --- a/scripts/rollup/validate/eslintrc.cjs2015.js +++ b/scripts/rollup/validate/eslintrc.cjs2015.js @@ -81,6 +81,17 @@ module.exports = { rules: { 'no-undef': 'error', 'no-shadow-restricted-names': 'error', + 'no-restricted-syntax': [ + 'error', + // TODO: Can be removed once we upgrade GCC to a version without `optimizeArgumentsArray` optimization. + { + selector: 'Identifier[name=/^JSCompiler_OptimizeArgumentsArray_/]', + message: + 'Google Closure Compiler optimized `arguments` access. ' + + 'This affects function arity. ' + + 'Access `arguments.length` to avoid this optimization', + }, + ], }, // These plugins aren't used, but eslint complains if an eslint-ignore comment diff --git a/scripts/rollup/validate/eslintrc.esm.js b/scripts/rollup/validate/eslintrc.esm.js index 9f9938f204d21..a8c5374f51990 100644 --- a/scripts/rollup/validate/eslintrc.esm.js +++ b/scripts/rollup/validate/eslintrc.esm.js @@ -83,6 +83,17 @@ module.exports = { rules: { 'no-undef': 'error', 'no-shadow-restricted-names': 'error', + 'no-restricted-syntax': [ + 'error', + // TODO: Can be removed once we upgrade GCC to a version without `optimizeArgumentsArray` optimization. + { + selector: 'Identifier[name=/^JSCompiler_OptimizeArgumentsArray_/]', + message: + 'Google Closure Compiler optimized `arguments` access. ' + + 'This affects function arity. ' + + 'Access `arguments.length` to avoid this optimization', + }, + ], }, // These plugins aren't used, but eslint complains if an eslint-ignore comment diff --git a/scripts/rollup/validate/eslintrc.fb.js b/scripts/rollup/validate/eslintrc.fb.js index 9f344c5aac3aa..f678723460080 100644 --- a/scripts/rollup/validate/eslintrc.fb.js +++ b/scripts/rollup/validate/eslintrc.fb.js @@ -71,6 +71,17 @@ module.exports = { rules: { 'no-undef': 'error', 'no-shadow-restricted-names': 'error', + 'no-restricted-syntax': [ + 'error', + // TODO: Can be removed once we upgrade GCC to a version without `optimizeArgumentsArray` optimization. + { + selector: 'Identifier[name=/^JSCompiler_OptimizeArgumentsArray_/]', + message: + 'Google Closure Compiler optimized `arguments` access. ' + + 'This affects function arity. ' + + 'Access `arguments.length` to avoid this optimization', + }, + ], }, // These plugins aren't used, but eslint complains if an eslint-ignore comment diff --git a/scripts/rollup/validate/eslintrc.rn.js b/scripts/rollup/validate/eslintrc.rn.js index d18516f802304..db6f12f75e9b7 100644 --- a/scripts/rollup/validate/eslintrc.rn.js +++ b/scripts/rollup/validate/eslintrc.rn.js @@ -73,6 +73,17 @@ module.exports = { rules: { 'no-undef': 'error', 'no-shadow-restricted-names': 'error', + 'no-restricted-syntax': [ + 'error', + // TODO: Can be removed once we upgrade GCC to a version without `optimizeArgumentsArray` optimization. + { + selector: 'Identifier[name=/^JSCompiler_OptimizeArgumentsArray_/]', + message: + 'Google Closure Compiler optimized `arguments` access. ' + + 'This affects function arity. ' + + 'Access `arguments.length` to avoid this optimization', + }, + ], }, // These plugins aren't used, but eslint complains if an eslint-ignore comment From bfc408c9cf3137784fb846c959d5635e5d1b3210 Mon Sep 17 00:00:00 2001 From: David Sancho Moreno Date: Mon, 16 Dec 2024 18:21:06 +0100 Subject: [PATCH 2/5] Annotate with nocollapse to avoid gcc argument optimization --- packages/react-dom/src/client/ReactDOMRoot.js | 12 ++++++++---- packages/react-reconciler/src/ReactFiberHooks.js | 8 ++++++-- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/packages/react-dom/src/client/ReactDOMRoot.js b/packages/react-dom/src/client/ReactDOMRoot.js index 1c673bf2543a0..4c0355a2a0ca3 100644 --- a/packages/react-dom/src/client/ReactDOMRoot.js +++ b/packages/react-dom/src/client/ReactDOMRoot.js @@ -106,17 +106,19 @@ ReactDOMHydrationRoot.prototype.render = ReactDOMRoot.prototype.render = } if (__DEV__) { - if (typeof arguments[1] === 'function') { + // @nocollapse - avoid GCC optimizations affecting function arity + const args = arguments; + if (typeof args[1] === 'function') { console.error( 'does not support the second callback argument. ' + 'To execute a side effect after rendering, declare it in a component body with useEffect().', ); - } else if (isValidContainer(arguments[1])) { + } else if (isValidContainer(args[1])) { console.error( 'You passed a container to the second argument of root.render(...). ' + "You don't need to pass it again since you already passed it to create the root.", ); - } else if (typeof arguments[1] !== 'undefined') { + } else if (typeof args[1] !== 'undefined') { console.error( 'You passed a second argument to root.render(...) but it only accepts ' + 'one argument.', @@ -131,7 +133,9 @@ ReactDOMHydrationRoot.prototype.unmount = ReactDOMRoot.prototype.unmount = // $FlowFixMe[missing-this-annot] function (): void { if (__DEV__) { - if (typeof arguments[0] === 'function') { + // @nocollapse - avoid GCC optimizations affecting function arity + const args = arguments; + if (typeof args[0] === 'function') { console.error( 'does not support a callback argument. ' + 'To execute a side effect after rendering, declare it in a component body with useEffect().', diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 8affe4804b8ab..be2e280ba6e0f 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -3644,7 +3644,9 @@ function dispatchReducerAction( action: A, ): void { if (__DEV__) { - if (typeof arguments[3] === 'function') { + // @nocollapse - avoid GCC optimizations affecting function arity + const args = arguments; + if (typeof args[3] === 'function') { console.error( "State updates from the useState() and useReducer() Hooks don't support the " + 'second callback argument. To execute a side effect after ' + @@ -3684,7 +3686,9 @@ function dispatchSetState( action: A, ): void { if (__DEV__) { - if (typeof arguments[3] === 'function') { + // @nocollapse - avoid GCC optimizations affecting function arity + const args = arguments; + if (typeof args[3] === 'function') { console.error( "State updates from the useState() and useReducer() Hooks don't support the " + 'second callback argument. To execute a side effect after ' + From 77864e0954d35c9137b93c94ec56acf55670b286 Mon Sep 17 00:00:00 2001 From: David Sancho Moreno Date: Mon, 16 Dec 2024 18:21:32 +0100 Subject: [PATCH 3/5] Add a test to ensure arity of setter in useState/useReducer --- .../react/src/__tests__/React-hooks-arity.js | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 packages/react/src/__tests__/React-hooks-arity.js diff --git a/packages/react/src/__tests__/React-hooks-arity.js b/packages/react/src/__tests__/React-hooks-arity.js new file mode 100644 index 0000000000000..0ba63428d3989 --- /dev/null +++ b/packages/react/src/__tests__/React-hooks-arity.js @@ -0,0 +1,44 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @emails react-core + */ + +'use strict'; + +let React; +let ReactNoop; + +describe('arity', () => { + beforeEach(() => { + jest.resetModules(); + + React = require('react'); + ReactNoop = require('react-noop-renderer'); + }); + + it("ensure useState setter's arity is correct", () => { + function Component() { + const [, setState] = React.useState(() => 'Halo!'); + + expect(setState.length).toBe(1); + return null; + } + + ReactNoop.render(); + }); + + it("ensure useReducer setter's arity is correct", () => { + function Component() { + const [, dispatch] = React.useReducer(() => 'Halo!'); + + expect(dispatch.length).toBe(1); + return null; + } + + ReactNoop.render(); + }); +}); From 9455c7cc93347c98d132538c4b0d03f090899e21 Mon Sep 17 00:00:00 2001 From: David Sancho Moreno Date: Mon, 16 Dec 2024 19:08:41 +0100 Subject: [PATCH 4/5] Replace // @nocollapse with a args reference --- packages/react-dom/src/client/ReactDOMRoot.js | 4 ++-- packages/react-reconciler/src/ReactFiberHooks.js | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/react-dom/src/client/ReactDOMRoot.js b/packages/react-dom/src/client/ReactDOMRoot.js index 4c0355a2a0ca3..5cef2d28437b0 100644 --- a/packages/react-dom/src/client/ReactDOMRoot.js +++ b/packages/react-dom/src/client/ReactDOMRoot.js @@ -106,7 +106,7 @@ ReactDOMHydrationRoot.prototype.render = ReactDOMRoot.prototype.render = } if (__DEV__) { - // @nocollapse - avoid GCC optimizations affecting function arity + // using a reference to `arguments` bails out of GCC optimizations which affect function arity const args = arguments; if (typeof args[1] === 'function') { console.error( @@ -133,7 +133,7 @@ ReactDOMHydrationRoot.prototype.unmount = ReactDOMRoot.prototype.unmount = // $FlowFixMe[missing-this-annot] function (): void { if (__DEV__) { - // @nocollapse - avoid GCC optimizations affecting function arity + // using a reference to `arguments` bails out of GCC optimizations which affect function arity const args = arguments; if (typeof args[0] === 'function') { console.error( diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index be2e280ba6e0f..284d6cabc85ee 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -3644,7 +3644,7 @@ function dispatchReducerAction( action: A, ): void { if (__DEV__) { - // @nocollapse - avoid GCC optimizations affecting function arity + // using a reference to `arguments` bails out of GCC optimizations which affect function arity const args = arguments; if (typeof args[3] === 'function') { console.error( @@ -3686,7 +3686,7 @@ function dispatchSetState( action: A, ): void { if (__DEV__) { - // @nocollapse - avoid GCC optimizations affecting function arity + // using a reference to `arguments` bails out of GCC optimizations which affect function arity const args = arguments; if (typeof args[3] === 'function') { console.error( From 926d19e27dc1005e15d0a9d0de5d6dac413d3ab7 Mon Sep 17 00:00:00 2001 From: David Sancho Moreno Date: Wed, 18 Dec 2024 12:30:45 +0100 Subject: [PATCH 5/5] Change message of no-restricted-syntax for gcc --- scripts/rollup/validate/eslintrc.cjs.js | 2 +- scripts/rollup/validate/eslintrc.cjs2015.js | 2 +- scripts/rollup/validate/eslintrc.esm.js | 2 +- scripts/rollup/validate/eslintrc.fb.js | 2 +- scripts/rollup/validate/eslintrc.rn.js | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/scripts/rollup/validate/eslintrc.cjs.js b/scripts/rollup/validate/eslintrc.cjs.js index d685efdd97b39..b974ecee0d903 100644 --- a/scripts/rollup/validate/eslintrc.cjs.js +++ b/scripts/rollup/validate/eslintrc.cjs.js @@ -94,7 +94,7 @@ module.exports = { message: 'Google Closure Compiler optimized `arguments` access. ' + 'This affects function arity. ' + - 'Access `arguments.length` to avoid this optimization', + 'Create a reference to `arguments` to avoid this optimization', }, ], }, diff --git a/scripts/rollup/validate/eslintrc.cjs2015.js b/scripts/rollup/validate/eslintrc.cjs2015.js index dfa6a9cd07934..6efc8838f0326 100644 --- a/scripts/rollup/validate/eslintrc.cjs2015.js +++ b/scripts/rollup/validate/eslintrc.cjs2015.js @@ -89,7 +89,7 @@ module.exports = { message: 'Google Closure Compiler optimized `arguments` access. ' + 'This affects function arity. ' + - 'Access `arguments.length` to avoid this optimization', + 'Create a reference to `arguments` to avoid this optimization', }, ], }, diff --git a/scripts/rollup/validate/eslintrc.esm.js b/scripts/rollup/validate/eslintrc.esm.js index a8c5374f51990..20b5341a82ad8 100644 --- a/scripts/rollup/validate/eslintrc.esm.js +++ b/scripts/rollup/validate/eslintrc.esm.js @@ -91,7 +91,7 @@ module.exports = { message: 'Google Closure Compiler optimized `arguments` access. ' + 'This affects function arity. ' + - 'Access `arguments.length` to avoid this optimization', + 'Create a reference to `arguments` to avoid this optimization', }, ], }, diff --git a/scripts/rollup/validate/eslintrc.fb.js b/scripts/rollup/validate/eslintrc.fb.js index f678723460080..9606d00b353ac 100644 --- a/scripts/rollup/validate/eslintrc.fb.js +++ b/scripts/rollup/validate/eslintrc.fb.js @@ -79,7 +79,7 @@ module.exports = { message: 'Google Closure Compiler optimized `arguments` access. ' + 'This affects function arity. ' + - 'Access `arguments.length` to avoid this optimization', + 'Create a reference to `arguments` to avoid this optimization', }, ], }, diff --git a/scripts/rollup/validate/eslintrc.rn.js b/scripts/rollup/validate/eslintrc.rn.js index db6f12f75e9b7..941b1d1872efd 100644 --- a/scripts/rollup/validate/eslintrc.rn.js +++ b/scripts/rollup/validate/eslintrc.rn.js @@ -81,7 +81,7 @@ module.exports = { message: 'Google Closure Compiler optimized `arguments` access. ' + 'This affects function arity. ' + - 'Access `arguments.length` to avoid this optimization', + 'Create a reference to `arguments` to avoid this optimization', }, ], },