Skip to content

Commit

Permalink
Make StaticViewConfigValidator permissive of extra attributes on SVC …
Browse files Browse the repository at this point in the history
…compared to native viewconfig (#45859)

Summary:
Pull Request resolved: #45859

"Fabric without SVCs" configuration is nearly gone, and so it doesn't make sense to need to add no-op methods, on normally Paper only code, etc to satisfy native viewconfig. These particular warnings are then more often noise, than things we need to action on.

Checking for native code to be present can also break development where users are using distributed native app, slightly older than JS.

This keeps the warning, only if static viewconfigs are missing native view config attributes (i.e. new prop would only be exposed to Paper, instead of only exposed to Fabric)

Changelog: [Internal]

Reviewed By: javache

Differential Revision: D60575253

fbshipit-source-id: 1c118274b92eb7922c0dd92df060b24e44fceb3d
  • Loading branch information
NickGerleman authored and facebook-github-bot committed Aug 21, 2024
1 parent a949e0d commit 387560a
Show file tree
Hide file tree
Showing 9 changed files with 44 additions and 313 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import type {
import getNativeComponentAttributes from '../ReactNative/getNativeComponentAttributes';
import UIManager from '../ReactNative/UIManager';
import * as ReactNativeViewConfigRegistry from '../Renderer/shims/ReactNativeViewConfigRegistry';
import verifyComponentAttributeEquivalence from '../Utilities/verifyComponentAttributeEquivalence';
import * as StaticViewConfigValidator from './StaticViewConfigValidator';
import {createViewConfig} from './ViewConfig';
import invariant from 'invariant';
Expand All @@ -35,7 +34,6 @@ let getRuntimeConfig;
export function setRuntimeConfigProvider(
runtimeConfigProvider: (name: string) => ?{
native: boolean,
strict: boolean,
verify: boolean,
},
): void {
Expand All @@ -55,9 +53,8 @@ export function get<Config>(
viewConfigProvider: () => PartialViewConfig,
): HostComponent<Config> {
ReactNativeViewConfigRegistry.register(name, () => {
const {native, strict, verify} = getRuntimeConfig?.(name) ?? {
const {native, verify} = getRuntimeConfig?.(name) ?? {
native: !global.RN$Bridgeless,
strict: false,
verify: false,
};

Expand Down Expand Up @@ -92,23 +89,19 @@ export function get<Config>(
? createViewConfig(viewConfigProvider())
: viewConfig;

if (strict) {
const validationOutput = StaticViewConfigValidator.validate(
name,
nativeViewConfig,
staticViewConfig,
const validationOutput = StaticViewConfigValidator.validate(
name,
nativeViewConfig,
staticViewConfig,
);

if (validationOutput.type === 'invalid') {
console.error(
StaticViewConfigValidator.stringifyValidationResult(
name,
validationOutput,
),
);

if (validationOutput.type === 'invalid') {
console.error(
StaticViewConfigValidator.stringifyValidationResult(
name,
validationOutput,
),
);
}
} else {
verifyComponentAttributeEquivalence(nativeViewConfig, staticViewConfig);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,6 @@ export type Difference =
path: Array<string>,
nativeValue: mixed,
staticValue: mixed,
}
| {
type: 'unexpected',
path: Array<string>,
staticValue: mixed,
};

export type ValidationResult = ValidResult | InvalidResult;
Expand Down Expand Up @@ -90,8 +85,6 @@ export function stringifyValidationResult(
return `- '${path.join('.')}' is missing.`;
case 'unequal':
return `- '${path.join('.')}' is the wrong value.`;
case 'unexpected':
return `- '${path.join('.')}' is present but not expected to be.`;
}
}),
'',
Expand Down Expand Up @@ -145,20 +138,6 @@ function accumulateDifferences(
});
}
}

for (const staticKey in staticObject) {
if (
!nativeObject.hasOwnProperty(staticKey) &&
// $FlowFixMe[invalid-computed-prop]
!isIgnored(staticObject[staticKey])
) {
differences.push({
path: [...path, staticKey],
type: 'unexpected',
staticValue: staticObject[staticKey],
});
}
}
}

function ifObject(value: mixed): ?{...} {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ StaticViewConfigValidator: Invalid static view config for 'RCTView'.
);
});

test('fails for unexpected attributes', () => {
test('allows static viewconfigs to have more properties than native viewconfigs', () => {
const name = 'RCTView';
const nativeViewConfig = {
uiViewClassName: 'RCTView',
Expand All @@ -194,19 +194,13 @@ test('fails for unexpected attributes', () => {
},
};

expectSVCToNotMatchNVC(
const validationResult = StaticViewConfigValidator.validate(
name,
nativeViewConfig,
staticViewConfig,
`
StaticViewConfigValidator: Invalid static view config for 'RCTView'.
- 'validAttributes.style.height' is present but not expected to be.
- 'validAttributes.style.width' is present but not expected to be.
- 'validAttributes.collapsable' is present but not expected to be.
- 'validAttributes.nativeID' is present but not expected to be.
`.trimStart(),
);

expect(validationResult.type).toBe('valid');
});

function expectSVCToNotMatchNVC(
Expand Down

This file was deleted.

22 changes: 22 additions & 0 deletions packages/react-native/Libraries/Utilities/stringifyViewConfig.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/**
* 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
*/

export default function stringifyViewConfig(viewConfig: any): string {
return JSON.stringify(
viewConfig,
(key, val) => {
if (typeof val === 'function') {
return ${val.name}`;
}
return val;
},
2,
);
}

This file was deleted.

Loading

0 comments on commit 387560a

Please sign in to comment.