-
Notifications
You must be signed in to change notification settings - Fork 47.2k
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
Post-process build files for React Native to add generated signature and @nolint #26616
Post-process build files for React Native to add generated signature and @nolint #26616
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I understand correctly, this is changing the build artifacts that are generated by yarn build
? If so, I think the right place to do this is in the diff train workflow, when we sync to the Meta repo. That way, Meta specific needs are not leaked into the build outputs, and stay in the sync pipeline.
Comparing: 7b0642b...a0dc889 Critical size changesIncludes critical production bundles, as well as any change greater than 2%:
Significant size changesIncludes any change greater than 0.2%: Expand to show
|
@rickhanlonii these generated files are already |
ae56fe4
to
26c4e58
Compare
26c4e58
to
a0dc889
Compare
Both, in my mind. The sync script used to add that during the sync, so the equivalent layer of that is the diff train sync script. Concretely, we shouldn't see the meta specific comments in the |
…and @NOLINT (facebook#26616) ## Summary We're enabling a new mechanism to synchronize build files from `react` to `react-native`. That new mechanism doesn't post-process files, so we need to add that post-processing somewhere. This PR does that when generating the files in the first place, so the generated files in the `build` directory are ready to be committed in the `react-native` repository directly. This makes use of `signedsource` to avoid direct modifications of these files in the `react-native` repository, as well as `@noformat` and `@nolint` to prevent unactionable CI failures in that repository. ## How did you test this change? Generated build files for `react-native` before and after this change: ``` node ./scripts/rollup/build-all-release-channels.js react-native ``` Checked new contents. Relevant changes: ```diff diff --color -r build-before/react-native/implementations/ReactFabric-dev.fb.js build-after/react-native/implementations/ReactFabric-dev.fb.js 10c10 < * @generated --- > * @generated SignedSource<<03cef14e77b8250b567dfdf3b066085e>> diff --color -r build-before/react-native/implementations/ReactFabric-dev.js build-after/react-native/implementations/ReactFabric-dev.js 11c11 < * @generated --- > * @generated SignedSource<<e39eed38a363846ca9ee9b59a225683c>> diff --color -r build-before/react-native/implementations/ReactFabric-prod.fb.js build-after/react-native/implementations/ReactFabric-prod.fb.js 10c10 < * @generated --- > * @generated SignedSource<<f65efcd6a469d5f6fef1ce647e5ec09a>> diff --color -r build-before/react-native/implementations/ReactFabric-prod.js build-after/react-native/implementations/ReactFabric-prod.js 11c11 < * @generated --- > * @generated SignedSource<<cdd582aa889b1054b2c5faf412622b18>> diff --color -r build-before/react-native/implementations/ReactFabric-profiling.fb.js build-after/react-native/implementations/ReactFabric-profiling.fb.js 10c10 < * @generated --- > * @generated SignedSource<<81e74849b24f104882bd298f062be0fa>> diff --color -r build-before/react-native/implementations/ReactFabric-profiling.js build-after/react-native/implementations/ReactFabric-profiling.js 11c11 < * @generated --- > * @generated SignedSource<<c050b7fa1453dc21ac1c5b98146210a8>> diff --color -r build-before/react-native/implementations/ReactNativeRenderer-dev.fb.js build-after/react-native/implementations/ReactNativeRenderer-dev.fb.js 10c10 < * @generated --- > * @generated SignedSource<<9c03464b489b41c06a065aeba8619263>> diff --color -r build-before/react-native/implementations/ReactNativeRenderer-dev.js build-after/react-native/implementations/ReactNativeRenderer-dev.js 11c11 < * @generated --- > * @generated SignedSource<<18b34c037544949dcf9b28f945921ba8>> diff --color -r build-before/react-native/implementations/ReactNativeRenderer-prod.fb.js build-after/react-native/implementations/ReactNativeRenderer-prod.fb.js 10c10 < * @generated --- > * @generated SignedSource<<592e9654c584d1da523378b119bd8bd7>> diff --color -r build-before/react-native/implementations/ReactNativeRenderer-prod.js build-after/react-native/implementations/ReactNativeRenderer-prod.js 11c11 < * @generated --- > * @generated SignedSource<<91c894db99e2d76f8a32708ad6ad1bde>> diff --color -r build-before/react-native/implementations/ReactNativeRenderer-profiling.fb.js build-after/react-native/implementations/ReactNativeRenderer-profiling.fb.js 10c10 < * @generated --- > * @generated SignedSource<<5ce378a9216ea747d91b208b9fd1ebd5>> diff --color -r build-before/react-native/implementations/ReactNativeRenderer-profiling.js build-after/react-native/implementations/ReactNativeRenderer-profiling.js 11c11 < * @generated --- > * @generated SignedSource<<1c7564f446ee83142976035b2884dcfd>> diff --color -r build-before/react-native/shims/ReactFabric.js build-after/react-native/shims/ReactFabric.js 7c7 < * @Format --- > * @noformat 8a9,10 > * @NOLINT > * @generated SignedSource<<cece19ddbec9f287c995721f49c68977>> diff --color -r build-before/react-native/shims/ReactFeatureFlags.js build-after/react-native/shims/ReactFeatureFlags.js 7c7 < * @Format --- > * @noformat 8a9,10 > * @NOLINT > * @generated SignedSource<<2881c8e89ef0f73f4cf6612cb518b197>> diff --color -r build-before/react-native/shims/ReactNative.js build-after/react-native/shims/ReactNative.js 7c7 < * @Format --- > * @noformat 8a9,10 > * @NOLINT > * @generated SignedSource<<0debd6e5a17dc037cb4661315a886de6>> diff --color -r build-before/react-native/shims/ReactNativeTypes.js build-after/react-native/shims/ReactNativeTypes.js 7c7 < * @Format --- > * @noformat 8a9,10 > * @NOLINT > * @generated SignedSource<<652b117c94307244bcf5e4af18928903>> diff --color -r build-before/react-native/shims/ReactNativeViewConfigRegistry.js build-after/react-native/shims/ReactNativeViewConfigRegistry.js 7c7 < * @Format --- > * @noformat 8a9,10 > * @NOLINT > * @generated SignedSource<<ce82e8957367bee7d11379ab88e3f7c5>> diff --color -r build-before/react-native/shims/createReactNativeComponentClass.js build-after/react-native/shims/createReactNativeComponentClass.js 7c7 < * @Format --- > * @noformat 8a9,10 > * @NOLINT > * @generated SignedSource<<ede54ac2fa1b9a09e234cdf098048989>> ```
This PR seems to have broken my ability to build React locally :( Context:
I just tried to merge
This happens with both It appears that it's coming from these steps in async function copyRNShims() {
await asyncCopyTo(
`${__dirname}/shims/react-native`,
'build/react-native/shims'
);
await asyncCopyTo(
require.resolve('react-native-renderer/src/ReactNativeTypes.js'),
'build/react-native/shims/ReactNativeTypes.js'
);
processGenerated('build/react-native/shims');
}
function processGenerated(directory) {
const files = readdirSync(directory)
.filter(dir => dir.endsWith('.js'))
.map(file => path.join(directory, file));
files.forEach(file => {
const originalContents = readFileSync(file, 'utf8');
const contents = originalContents
// Replace {@}format with {@}noformat
.replace(/(\n\s*\*\s*)@format\b.*(\n)/, '$1@noformat$2')
// Add {@}nolint and {@}generated
.replace(' */\n', ` * @nolint\n * ${getSigningToken()}\n */\n`);
const signedContents = signFile(contents);
writeFileSync(file, signedContents, 'utf8');
});
} and specifically from trying to process /**
* 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.
*
* @format
* @flow strict-local
*/
'use strict';
import {ReactNativeViewConfigRegistry} from 'react-native/Libraries/ReactPrivate/ReactNativePrivateInterface';
import {type ViewConfig} from './ReactNativeTypes';
const {register} = ReactNativeViewConfigRegistry;
/**
* Creates a renderable ReactNative host component.
* Use this method for view configs that are loaded from UIManager.
* Use createReactNativeComponentClass() for view configs defined within JavaScript.
*
* @param {string} config iOS View configuration.
* @private
*/
const createReactNativeComponentClass = function (
name: string,
callback: () => ViewConfig,
): string {
return register(name, callback);
};
module.exports = createReactNativeComponentClass; Given that the shim file doesn't have a token-related string in it, I think the error sort of makes sense conceptually. I'm not sure what would be missing or different on my end that would make this fail, or why no one else seems to have reported it yet. Any suggestions on getting the build process working for me again? |
Someone else is seeing the same issue in #26697 |
Thanks for reporting @markerikson! I think this is a Windows-specific issue related to line endings. Working on a fix now. I'll follow up on #26697. |
## Summary We added some post-processing in the build for RN in #26616 that broke for users on Windows due to how line endings were handled to the regular expression to insert some directives in the docblock. This fixes that problem, reported in #26697 as well. ## How did you test this change? Verified files are still built correctly on Mac/Linux. Will ask for help to test on Windows.
## Summary We added some post-processing in the build for RN in #26616 that broke for users on Windows due to how line endings were handled to the regular expression to insert some directives in the docblock. This fixes that problem, reported in #26697 as well. ## How did you test this change? Verified files are still built correctly on Mac/Linux. Will ask for help to test on Windows. DiffTrain build for [f87e97a](f87e97a)
…and @NOLINT (facebook#26616) ## Summary We're enabling a new mechanism to synchronize build files from `react` to `react-native`. That new mechanism doesn't post-process files, so we need to add that post-processing somewhere. This PR does that when generating the files in the first place, so the generated files in the `build` directory are ready to be committed in the `react-native` repository directly. This makes use of `signedsource` to avoid direct modifications of these files in the `react-native` repository, as well as `@noformat` and `@nolint` to prevent unactionable CI failures in that repository. ## How did you test this change? Generated build files for `react-native` before and after this change: ``` node ./scripts/rollup/build-all-release-channels.js react-native ``` Checked new contents. Relevant changes: ```diff diff --color -r build-before/react-native/implementations/ReactFabric-dev.fb.js build-after/react-native/implementations/ReactFabric-dev.fb.js 10c10 < * @generated --- > * @generated SignedSource<<03cef14e77b8250b567dfdf3b066085e>> diff --color -r build-before/react-native/implementations/ReactFabric-dev.js build-after/react-native/implementations/ReactFabric-dev.js 11c11 < * @generated --- > * @generated SignedSource<<e39eed38a363846ca9ee9b59a225683c>> diff --color -r build-before/react-native/implementations/ReactFabric-prod.fb.js build-after/react-native/implementations/ReactFabric-prod.fb.js 10c10 < * @generated --- > * @generated SignedSource<<f65efcd6a469d5f6fef1ce647e5ec09a>> diff --color -r build-before/react-native/implementations/ReactFabric-prod.js build-after/react-native/implementations/ReactFabric-prod.js 11c11 < * @generated --- > * @generated SignedSource<<cdd582aa889b1054b2c5faf412622b18>> diff --color -r build-before/react-native/implementations/ReactFabric-profiling.fb.js build-after/react-native/implementations/ReactFabric-profiling.fb.js 10c10 < * @generated --- > * @generated SignedSource<<81e74849b24f104882bd298f062be0fa>> diff --color -r build-before/react-native/implementations/ReactFabric-profiling.js build-after/react-native/implementations/ReactFabric-profiling.js 11c11 < * @generated --- > * @generated SignedSource<<c050b7fa1453dc21ac1c5b98146210a8>> diff --color -r build-before/react-native/implementations/ReactNativeRenderer-dev.fb.js build-after/react-native/implementations/ReactNativeRenderer-dev.fb.js 10c10 < * @generated --- > * @generated SignedSource<<9c03464b489b41c06a065aeba8619263>> diff --color -r build-before/react-native/implementations/ReactNativeRenderer-dev.js build-after/react-native/implementations/ReactNativeRenderer-dev.js 11c11 < * @generated --- > * @generated SignedSource<<18b34c037544949dcf9b28f945921ba8>> diff --color -r build-before/react-native/implementations/ReactNativeRenderer-prod.fb.js build-after/react-native/implementations/ReactNativeRenderer-prod.fb.js 10c10 < * @generated --- > * @generated SignedSource<<592e9654c584d1da523378b119bd8bd7>> diff --color -r build-before/react-native/implementations/ReactNativeRenderer-prod.js build-after/react-native/implementations/ReactNativeRenderer-prod.js 11c11 < * @generated --- > * @generated SignedSource<<91c894db99e2d76f8a32708ad6ad1bde>> diff --color -r build-before/react-native/implementations/ReactNativeRenderer-profiling.fb.js build-after/react-native/implementations/ReactNativeRenderer-profiling.fb.js 10c10 < * @generated --- > * @generated SignedSource<<5ce378a9216ea747d91b208b9fd1ebd5>> diff --color -r build-before/react-native/implementations/ReactNativeRenderer-profiling.js build-after/react-native/implementations/ReactNativeRenderer-profiling.js 11c11 < * @generated --- > * @generated SignedSource<<1c7564f446ee83142976035b2884dcfd>> diff --color -r build-before/react-native/shims/ReactFabric.js build-after/react-native/shims/ReactFabric.js 7c7 < * @Format --- > * @noformat 8a9,10 > * @NOLINT > * @generated SignedSource<<cece19ddbec9f287c995721f49c68977>> diff --color -r build-before/react-native/shims/ReactFeatureFlags.js build-after/react-native/shims/ReactFeatureFlags.js 7c7 < * @Format --- > * @noformat 8a9,10 > * @NOLINT > * @generated SignedSource<<2881c8e89ef0f73f4cf6612cb518b197>> diff --color -r build-before/react-native/shims/ReactNative.js build-after/react-native/shims/ReactNative.js 7c7 < * @Format --- > * @noformat 8a9,10 > * @NOLINT > * @generated SignedSource<<0debd6e5a17dc037cb4661315a886de6>> diff --color -r build-before/react-native/shims/ReactNativeTypes.js build-after/react-native/shims/ReactNativeTypes.js 7c7 < * @Format --- > * @noformat 8a9,10 > * @NOLINT > * @generated SignedSource<<652b117c94307244bcf5e4af18928903>> diff --color -r build-before/react-native/shims/ReactNativeViewConfigRegistry.js build-after/react-native/shims/ReactNativeViewConfigRegistry.js 7c7 < * @Format --- > * @noformat 8a9,10 > * @NOLINT > * @generated SignedSource<<ce82e8957367bee7d11379ab88e3f7c5>> diff --color -r build-before/react-native/shims/createReactNativeComponentClass.js build-after/react-native/shims/createReactNativeComponentClass.js 7c7 < * @Format --- > * @noformat 8a9,10 > * @NOLINT > * @generated SignedSource<<ede54ac2fa1b9a09e234cdf098048989>> ```
…book#26727) ## Summary We added some post-processing in the build for RN in facebook#26616 that broke for users on Windows due to how line endings were handled to the regular expression to insert some directives in the docblock. This fixes that problem, reported in facebook#26697 as well. ## How did you test this change? Verified files are still built correctly on Mac/Linux. Will ask for help to test on Windows.
…and @NOLINT (#26616) ## Summary We're enabling a new mechanism to synchronize build files from `react` to `react-native`. That new mechanism doesn't post-process files, so we need to add that post-processing somewhere. This PR does that when generating the files in the first place, so the generated files in the `build` directory are ready to be committed in the `react-native` repository directly. This makes use of `signedsource` to avoid direct modifications of these files in the `react-native` repository, as well as `@noformat` and `@nolint` to prevent unactionable CI failures in that repository. ## How did you test this change? Generated build files for `react-native` before and after this change: ``` node ./scripts/rollup/build-all-release-channels.js react-native ``` Checked new contents. Relevant changes: ```diff diff --color -r build-before/react-native/implementations/ReactFabric-dev.fb.js build-after/react-native/implementations/ReactFabric-dev.fb.js 10c10 < * @generated --- > * @generated SignedSource<<03cef14e77b8250b567dfdf3b066085e>> diff --color -r build-before/react-native/implementations/ReactFabric-dev.js build-after/react-native/implementations/ReactFabric-dev.js 11c11 < * @generated --- > * @generated SignedSource<<e39eed38a363846ca9ee9b59a225683c>> diff --color -r build-before/react-native/implementations/ReactFabric-prod.fb.js build-after/react-native/implementations/ReactFabric-prod.fb.js 10c10 < * @generated --- > * @generated SignedSource<<f65efcd6a469d5f6fef1ce647e5ec09a>> diff --color -r build-before/react-native/implementations/ReactFabric-prod.js build-after/react-native/implementations/ReactFabric-prod.js 11c11 < * @generated --- > * @generated SignedSource<<cdd582aa889b1054b2c5faf412622b18>> diff --color -r build-before/react-native/implementations/ReactFabric-profiling.fb.js build-after/react-native/implementations/ReactFabric-profiling.fb.js 10c10 < * @generated --- > * @generated SignedSource<<81e74849b24f104882bd298f062be0fa>> diff --color -r build-before/react-native/implementations/ReactFabric-profiling.js build-after/react-native/implementations/ReactFabric-profiling.js 11c11 < * @generated --- > * @generated SignedSource<<c050b7fa1453dc21ac1c5b98146210a8>> diff --color -r build-before/react-native/implementations/ReactNativeRenderer-dev.fb.js build-after/react-native/implementations/ReactNativeRenderer-dev.fb.js 10c10 < * @generated --- > * @generated SignedSource<<9c03464b489b41c06a065aeba8619263>> diff --color -r build-before/react-native/implementations/ReactNativeRenderer-dev.js build-after/react-native/implementations/ReactNativeRenderer-dev.js 11c11 < * @generated --- > * @generated SignedSource<<18b34c037544949dcf9b28f945921ba8>> diff --color -r build-before/react-native/implementations/ReactNativeRenderer-prod.fb.js build-after/react-native/implementations/ReactNativeRenderer-prod.fb.js 10c10 < * @generated --- > * @generated SignedSource<<592e9654c584d1da523378b119bd8bd7>> diff --color -r build-before/react-native/implementations/ReactNativeRenderer-prod.js build-after/react-native/implementations/ReactNativeRenderer-prod.js 11c11 < * @generated --- > * @generated SignedSource<<91c894db99e2d76f8a32708ad6ad1bde>> diff --color -r build-before/react-native/implementations/ReactNativeRenderer-profiling.fb.js build-after/react-native/implementations/ReactNativeRenderer-profiling.fb.js 10c10 < * @generated --- > * @generated SignedSource<<5ce378a9216ea747d91b208b9fd1ebd5>> diff --color -r build-before/react-native/implementations/ReactNativeRenderer-profiling.js build-after/react-native/implementations/ReactNativeRenderer-profiling.js 11c11 < * @generated --- > * @generated SignedSource<<1c7564f446ee83142976035b2884dcfd>> diff --color -r build-before/react-native/shims/ReactFabric.js build-after/react-native/shims/ReactFabric.js 7c7 < * @Format --- > * @noformat 8a9,10 > * @NOLINT > * @generated SignedSource<<cece19ddbec9f287c995721f49c68977>> diff --color -r build-before/react-native/shims/ReactFeatureFlags.js build-after/react-native/shims/ReactFeatureFlags.js 7c7 < * @Format --- > * @noformat 8a9,10 > * @NOLINT > * @generated SignedSource<<2881c8e89ef0f73f4cf6612cb518b197>> diff --color -r build-before/react-native/shims/ReactNative.js build-after/react-native/shims/ReactNative.js 7c7 < * @Format --- > * @noformat 8a9,10 > * @NOLINT > * @generated SignedSource<<0debd6e5a17dc037cb4661315a886de6>> diff --color -r build-before/react-native/shims/ReactNativeTypes.js build-after/react-native/shims/ReactNativeTypes.js 7c7 < * @Format --- > * @noformat 8a9,10 > * @NOLINT > * @generated SignedSource<<652b117c94307244bcf5e4af18928903>> diff --color -r build-before/react-native/shims/ReactNativeViewConfigRegistry.js build-after/react-native/shims/ReactNativeViewConfigRegistry.js 7c7 < * @Format --- > * @noformat 8a9,10 > * @NOLINT > * @generated SignedSource<<ce82e8957367bee7d11379ab88e3f7c5>> diff --color -r build-before/react-native/shims/createReactNativeComponentClass.js build-after/react-native/shims/createReactNativeComponentClass.js 7c7 < * @Format --- > * @noformat 8a9,10 > * @NOLINT > * @generated SignedSource<<ede54ac2fa1b9a09e234cdf098048989>> ``` DiffTrain build for commit 39a3b72.
## Summary We added some post-processing in the build for RN in #26616 that broke for users on Windows due to how line endings were handled to the regular expression to insert some directives in the docblock. This fixes that problem, reported in #26697 as well. ## How did you test this change? Verified files are still built correctly on Mac/Linux. Will ask for help to test on Windows. DiffTrain build for commit f87e97a.
## Overview Reverts #26616 and implements the suggested way instead. This change in #26616 broken the internal sync command, which now results in duplicated `@generated` headers. It also makes it harder to detect changes during the diff train sync. Instead, we will check for changes, and if there are changes sign the files and commit them to the sync branch. ## Strategy The new sync strategy accounts for the generated headers during the sync: - **Revert Version**: Revert the version strings - **Revert @generated**: Re-sign the files (will be the same hash as before if unchanged) - **Check**: Check if there are changes **if not, skip** - **Re-apply Version**: Now add back the new version string - **Re-sign @generated**: And re-generate the headers Then commit to branch. This ensures that if there are no changes, we'll skip. --------- Co-authored-by: Timothy Yung <yungsters@gmail.com>
Summary
We're enabling a new mechanism to synchronize build files from
react
toreact-native
. That new mechanism doesn't post-process files, so we need to add that post-processing somewhere. This PR does that when generating the files in the first place, so the generated files in thebuild
directory are ready to be committed in thereact-native
repository directly.This makes use of
signedsource
to avoid direct modifications of these files in thereact-native
repository, as well as@noformat
and@nolint
to prevent unactionable CI failures in that repository.How did you test this change?
Generated build files for
react-native
before and after this change:Checked new contents. Relevant changes: