Separate component array types and command array types #48474
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
facebook-github-bot
added
CLA Signed
|
This pull request was exported from Phabricator. Differential Revision: D67806818 |
elicwhite
force-pushed
the
export-D67806818
branch
from
92817ef to
8b2f4e3
Compare
elicwhite
added a commit
to elicwhite/react-native
that referenced
this pull request
Jan 3, 2025
Summary:
The previous definition said that you could put prop types into commands, which definitely isn't allowed.
For example, the schema would have allowed `WithDefault` types.
```
interface NativeCommands {
+methodInt: (viewRef: React.ElementRef<NativeType>, a: WithDefault<string, 'hi'>) => void;
}
```
This change separates out the things that are allowed in commands from what's allowed in props.
Commands should be very similar to what's allowed in native modules, but it isn't exact enough to be able to merge those.
Differential Revision: D67806818
|
This pull request was exported from Phabricator. Differential Revision: D67806818 |
elicwhite
added a commit
to elicwhite/react-native
that referenced
this pull request
Jan 3, 2025
Summary:
The previous definition said that you could put prop types into commands, which definitely isn't allowed.
For example, the schema would have allowed `WithDefault` types.
```
interface NativeCommands {
+methodInt: (viewRef: React.ElementRef<NativeType>, a: WithDefault<string, 'hi'>) => void;
}
```
This change separates out the things that are allowed in commands from what's allowed in props.
Commands should be very similar to what's allowed in native modules, but it isn't exact enough to be able to merge those.
Differential Revision: D67806818
…es (facebook#48477) Summary: Over the years of copying and moving and renaming types through CodegenSchema, this type ended up in the Command params, although the Command parser doesn't allow it. I made this change to a fixture: {F1974104959} and got this error ``` FAIL xplat/js/react-native-github/packages/react-native-codegen/src/parsers/flow/components/__tests__/component-parser-test.js ● RN Codegen Flow Parser › can generate fixture COMMANDS_DEFINED_WITH_ALL_TYPES Unsupported param type for method "scrollTo", param "speed". Found UnionTypeAnnotation 127 | default: 128 | (type: empty); > 129 | throw new Error( | ^ 130 | `Unsupported param type for method "${name}", param "${paramName}". Found ${type}`, 131 | ); 132 | } ``` Also, a default value for enum an argument of a Command doesn't make sense anyways. Commands should probably have support for enums and string literal unions, but that's out of scope here. Still need to add to this vec\concat on www: https://www.internalfb.com/code/www/[ebfa58f888a6064e17879934d447f59bcc2b6951]/flib/intern/sandcastle/react_native/ota_steps/SandcastleOTACompatibilityCheckReportingStep.php?lines=62 Changelog: [internal] Differential Revision: D67806808
Summary: This is such a simpler approach lol. I'll need this later for when I want to pass in arrays or objects of these types to the compat check Changelog: [internal] Differential Revision: D67806812
Summary:
The previous definition said that you could put prop types into commands, which definitely isn't allowed.
For example, the schema would have allowed `WithDefault` types.
```
interface NativeCommands {
+methodInt: (viewRef: React.ElementRef<NativeType>, a: WithDefault<string, 'hi'>) => void;
}
```
This change separates out the things that are allowed in commands from what's allowed in props.
Commands should be very similar to what's allowed in native modules, but it isn't exact enough to be able to merge those.
Differential Revision: D67806818
elicwhite
force-pushed
the
export-D67806818
branch
from
8b2f4e3 to
cadb1d7
Compare
|
This pull request was exported from Phabricator. Differential Revision: D67806818 |
|
This pull request has been merged in 825492b. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
The previous definition said that you could put prop types into commands, which definitely isn't allowed.
For example, the schema would have allowed
WithDefaulttypes.This change separates out the things that are allowed in commands from what's allowed in props.
Commands should be very similar to what's allowed in native modules, but it isn't exact enough to be able to merge those.
Differential Revision: D67806818