From 9e18c5649f4de96bad6b29d9a16380d726dfe0d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Norte?= Date: Mon, 11 Mar 2024 03:43:32 -0700 Subject: [PATCH 1/2] [skip ci] Update feature flag generator to create directories for generated files before writing (#43393) Summary: Changelog: [internal] The generator doesn't create intermediate directories, which is causing issues now that we're moving the generated native module spec to a new directory. This fixes that. Differential Revision: D54690126 --- packages/react-native/scripts/featureflags/generateFiles.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/react-native/scripts/featureflags/generateFiles.js b/packages/react-native/scripts/featureflags/generateFiles.js index f03df2163e64..8f155b1350ae 100644 --- a/packages/react-native/scripts/featureflags/generateFiles.js +++ b/packages/react-native/scripts/featureflags/generateFiles.js @@ -14,6 +14,7 @@ import generateAndroidModules from './generateAndroidModules'; import generateCommonCxxModules from './generateCommonCxxModules'; import generateJavaScriptModules from './generateJavaScriptModules'; import fs from 'fs'; +import path from 'path'; export default function generateFiles( generatorConfig: GeneratorConfig, @@ -65,6 +66,7 @@ export default function generateFiles( } for (const [modulePath, moduleContents] of Object.entries(generatedFiles)) { - fs.writeFileSync(modulePath, moduleContents, 'utf8'); + fs.mkdirSync(path.dirname(modulePath), {recursive: true}); + fs.writeFileSync(modulePath, moduleContents, {encoding: 'utf8'}); } } From 212b7613d59626b08e0007fb3c1d782ba430741f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Norte?= Date: Mon, 11 Mar 2024 03:43:32 -0700 Subject: [PATCH 2/2] Allow accessing common feature flags before setting overrides for JS-only overrides (#43394) Summary: Changelog: [internal] When we built the new feature flag system we added a constraint in the JS API to prevent calling `override` if any of the flags was already accessed from JS. This is very restrictive because it doesn't allow us to access common flags (like `enableMicrotasks`) set up from native during the initialization of the framework because then applications wouldn't be able to set JS-only overrides. This relaxes the constraint to disallow accessing JS-only flags before setting JS-only overrides, but accessing common (native) flags before that is ok. Differential Revision: D54687055 --- .../ReactNativeFeatureFlagsBase.js | 6 ++++-- .../__tests__/ReactNativeFeatureFlags-test.js | 20 ++++++++++++++++--- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/packages/react-native/src/private/featureflags/ReactNativeFeatureFlagsBase.js b/packages/react-native/src/private/featureflags/ReactNativeFeatureFlagsBase.js index 3223800bed4a..d7398c5358ca 100644 --- a/packages/react-native/src/private/featureflags/ReactNativeFeatureFlagsBase.js +++ b/packages/react-native/src/private/featureflags/ReactNativeFeatureFlagsBase.js @@ -29,7 +29,6 @@ function createGetter( return () => { if (cachedValue == null) { - accessedFeatureFlags.add(configName); cachedValue = customValueGetter() ?? defaultValue; } return cachedValue; @@ -44,7 +43,10 @@ export function createJavaScriptFlagGetter< ): Getter> { return createGetter( configName, - () => overrides?.[configName]?.(), + () => { + accessedFeatureFlags.add(configName); + return overrides?.[configName]?.(); + }, defaultValue, ); } diff --git a/packages/react-native/src/private/featureflags/__tests__/ReactNativeFeatureFlags-test.js b/packages/react-native/src/private/featureflags/__tests__/ReactNativeFeatureFlags-test.js index 02d61696e10f..0b3ea72ec38b 100644 --- a/packages/react-native/src/private/featureflags/__tests__/ReactNativeFeatureFlags-test.js +++ b/packages/react-native/src/private/featureflags/__tests__/ReactNativeFeatureFlags-test.js @@ -84,20 +84,34 @@ describe('ReactNativeFeatureFlags', () => { expect(jsOnlyTestFlagFn).toHaveBeenCalledTimes(1); }); - it('should throw an error if any of the flags has been accessed before overridding', () => { + it('should throw an error if any of the JS flags has been accessed before overridding', () => { const ReactNativeFeatureFlags = require('../ReactNativeFeatureFlags'); - ReactNativeFeatureFlags.commonTestFlag(); + ReactNativeFeatureFlags.jsOnlyTestFlag(); expect(() => ReactNativeFeatureFlags.override({ jsOnlyTestFlag: () => true, }), ).toThrow( - 'Feature flags were accessed before being overridden: commonTestFlag', + 'Feature flags were accessed before being overridden: jsOnlyTestFlag', ); }); + it('should NOT throw an error if any of the common flags has been accessed before overridding', () => { + const ReactNativeFeatureFlags = require('../ReactNativeFeatureFlags'); + + ReactNativeFeatureFlags.commonTestFlag(); + + expect(() => { + ReactNativeFeatureFlags.override({ + jsOnlyTestFlag: () => true, + }); + }).not.toThrow(); + + expect(ReactNativeFeatureFlags.jsOnlyTestFlag()).toBe(true); + }); + it('should throw an error when trying to set overrides twice', () => { const ReactNativeFeatureFlags = require('../ReactNativeFeatureFlags');