Skip to content

Commit c4b094f

Browse files
gispadaDmitry Rykun
authored and
Dmitry Rykun
committed
Fix: Codegen template error in RCTThirdPartyFabricComponentsProvider (#34738)
Summary: When `GenerateRCTThirdPartyFabricComponentsProviderCpp.js` generates `RCTThirdPartyFabricComponentsProvider.mm` an edge case happens in the following situation: - The same library exports multiple modules with one component each (i.e. one component per file); - The **first component** is excluded for iOS via the `excludedPlatforms` property in *codegenNativeComponent*. A "loose" comma appears in the generated template, breaking the code. ```c++ Class<RCTComponentViewProtocol> RCTThirdPartyFabricComponentsProvider(const char *name) { static std::unordered_map<std::string, Class (*)(void)> sFabricComponentsClassMap = { , // <-- the offending comma {"NativeComponent2", NativeComponent2Cls}, // rnmylibrary }; } ``` At some point, `GenerateRCTThirdPartyFabricComponentsProviderCpp.js` does not properly filter out empty arrays resulting from excluded components. This does not seem to be a problem when the excluded component is not the first being processed, as the comma gets added at the end of the previous line, after the comment with the name of the library. ## Changelog <!-- Help reviewers and the release process by writing your own changelog entry. For an example, see: https://reactnative.dev/contributing/changelogs-in-pull-requests --> [iOS] [Fixed] - Fix error in the Codegen template for ThirdPartyFabricComponentsProvider Pull Request resolved: #34738 Test Plan: <!-- Demonstrate the code is solid. Example: The exact commands you ran and their output, screenshots / videos if the pull request changes the user interface. --> This is the schema that leads to the bug. Notice that the first component was excluded for iOS. ```json { "modules": { "ComponentFile1": { "type": "Component", "components": { "NativeComponent1": { "excludedPlatforms": ["iOS"] "extendsProps": [ { "type": "ReactNativeBuiltInType", "knownTypeName": "ReactNativeCoreViewProps" } ], "events": [], "props": [], "commands": [] } } }, "ComponentFile2": { "type": "Component", "components": { "NativeComponent2": { "extendsProps": [ { "type": "ReactNativeBuiltInType", "knownTypeName": "ReactNativeCoreViewProps" } ], "events": [], "props": [], "commands": [] } } } } ``` `GenerateRCTThirdPartyFabricComponentsProviderCpp.js` should generate a template without the comma in the wrong position (before NativeComponent2). I also added an additional test case to cover this problem. All the other tests passed. Reviewed By: sammy-SC Differential Revision: D39686573 Pulled By: cipolleschi fbshipit-source-id: 6054464d024218eb0b2e02974aa5cc7c8aebbbc9
1 parent 917e97b commit c4b094f

17 files changed

+571
-1
lines changed

packages/react-native-codegen/src/generators/components/GenerateThirdPartyFabricComponentsProviderObjCpp.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ module.exports = {
7575
return null;
7676
}
7777

78-
return Object.keys(components)
78+
const componentTemplates = Object.keys(components)
7979
.filter(componentName => {
8080
const component = components[componentName];
8181
return !(
@@ -91,6 +91,8 @@ module.exports = {
9191

9292
return replacedTemplate;
9393
});
94+
95+
return componentTemplates.length > 0 ? componentTemplates : null;
9496
})
9597
.filter(Boolean);
9698
})

packages/react-native-codegen/src/generators/components/__test_fixtures__/fixtures.js

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1594,6 +1594,53 @@ const EXCLUDE_ANDROID_IOS: SchemaType = {
15941594
},
15951595
};
15961596

1597+
const EXCLUDE_IOS_TWO_COMPONENTS_DIFFERENT_FILES: SchemaType = {
1598+
modules: {
1599+
ComponentFile1: {
1600+
type: 'Component',
1601+
components: {
1602+
ExcludedIosComponent: {
1603+
excludedPlatforms: ['iOS'],
1604+
extendsProps: [
1605+
{
1606+
type: 'ReactNativeBuiltInType',
1607+
knownTypeName: 'ReactNativeCoreViewProps',
1608+
},
1609+
],
1610+
events: [],
1611+
props: [],
1612+
commands: [],
1613+
},
1614+
},
1615+
},
1616+
ComponentFile2: {
1617+
type: 'Component',
1618+
components: {
1619+
MultiFileIncludedNativeComponent: {
1620+
extendsProps: [
1621+
{
1622+
type: 'ReactNativeBuiltInType',
1623+
knownTypeName: 'ReactNativeCoreViewProps',
1624+
},
1625+
],
1626+
events: [],
1627+
props: [
1628+
{
1629+
name: 'disabled',
1630+
optional: true,
1631+
typeAnnotation: {
1632+
type: 'BooleanTypeAnnotation',
1633+
default: true,
1634+
},
1635+
},
1636+
],
1637+
commands: [],
1638+
},
1639+
},
1640+
},
1641+
},
1642+
};
1643+
15971644
module.exports = {
15981645
NO_PROPS_NO_EVENTS,
15991646
INTERFACE_ONLY,
@@ -1621,4 +1668,5 @@ module.exports = {
16211668
COMMANDS_AND_PROPS,
16221669
EXCLUDE_ANDROID,
16231670
EXCLUDE_ANDROID_IOS,
1671+
EXCLUDE_IOS_TWO_COMPONENTS_DIFFERENT_FILES,
16241672
};

packages/react-native-codegen/src/generators/components/__tests__/__snapshots__/GenerateComponentDescriptorH-test.js.snap

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -336,6 +336,35 @@ using ExcludedAndroidIosComponentComponentDescriptor = ConcreteComponentDescript
336336
}
337337
`;
338338
339+
exports[`GenerateComponentDescriptorH can generate fixture EXCLUDE_IOS_TWO_COMPONENTS_DIFFERENT_FILES 1`] = `
340+
Map {
341+
"ComponentDescriptors.h" => "
342+
/**
343+
* This code was generated by [react-native-codegen](https://www.npmjs.com/package/react-native-codegen).
344+
*
345+
* Do not edit this file as changes may cause incorrect behavior and will be lost
346+
* once the code is regenerated.
347+
*
348+
* @generated by codegen project: GenerateComponentDescriptorH.js
349+
*/
350+
351+
#pragma once
352+
353+
#include <react/renderer/components/EXCLUDE_IOS_TWO_COMPONENTS_DIFFERENT_FILES/ShadowNodes.h>
354+
#include <react/renderer/core/ConcreteComponentDescriptor.h>
355+
356+
namespace facebook {
357+
namespace react {
358+
359+
using ExcludedIosComponentComponentDescriptor = ConcreteComponentDescriptor<ExcludedIosComponentShadowNode>;
360+
using MultiFileIncludedNativeComponentComponentDescriptor = ConcreteComponentDescriptor<MultiFileIncludedNativeComponentShadowNode>;
361+
362+
} // namespace react
363+
} // namespace facebook
364+
",
365+
}
366+
`;
367+
339368
exports[`GenerateComponentDescriptorH can generate fixture FLOAT_PROPS 1`] = `
340369
Map {
341370
"ComponentDescriptors.h" => "

packages/react-native-codegen/src/generators/components/__tests__/__snapshots__/GenerateComponentHObjCpp-test.js.snap

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -430,6 +430,31 @@ NS_ASSUME_NONNULL_BEGIN
430430
431431
432432
433+
NS_ASSUME_NONNULL_END",
434+
}
435+
`;
436+
437+
exports[`GenerateComponentHObjCpp can generate fixture EXCLUDE_IOS_TWO_COMPONENTS_DIFFERENT_FILES 1`] = `
438+
Map {
439+
"RCTComponentViewHelpers.h" => "/**
440+
* This code was generated by [react-native-codegen](https://www.npmjs.com/package/react-native-codegen).
441+
*
442+
* Do not edit this file as changes may cause incorrect behavior and will be lost
443+
* once the code is regenerated.
444+
*
445+
* @generated by codegen project: GenerateComponentHObjCpp.js
446+
*/
447+
448+
#import <Foundation/Foundation.h>
449+
#import <React/RCTDefines.h>
450+
#import <React/RCTLog.h>
451+
452+
NS_ASSUME_NONNULL_BEGIN
453+
454+
@protocol RCTMultiFileIncludedNativeComponentViewProtocol <NSObject>
455+
456+
@end
457+
433458
NS_ASSUME_NONNULL_END",
434459
}
435460
`;

packages/react-native-codegen/src/generators/components/__tests__/__snapshots__/GenerateEventEmitterCpp-test.js.snap

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -351,6 +351,32 @@ namespace react {
351351
352352
353353
354+
} // namespace react
355+
} // namespace facebook
356+
",
357+
}
358+
`;
359+
360+
exports[`GenerateEventEmitterCpp can generate fixture EXCLUDE_IOS_TWO_COMPONENTS_DIFFERENT_FILES 1`] = `
361+
Map {
362+
"EventEmitters.cpp" => "
363+
/**
364+
* This code was generated by [react-native-codegen](https://www.npmjs.com/package/react-native-codegen).
365+
*
366+
* Do not edit this file as changes may cause incorrect behavior and will be lost
367+
* once the code is regenerated.
368+
*
369+
* @generated by codegen project: GenerateEventEmitterCpp.js
370+
*/
371+
372+
#include <react/renderer/components/EXCLUDE_IOS_TWO_COMPONENTS_DIFFERENT_FILES/EventEmitters.h>
373+
374+
namespace facebook {
375+
namespace react {
376+
377+
378+
379+
354380
} // namespace react
355381
} // namespace facebook
356382
",

packages/react-native-codegen/src/generators/components/__tests__/__snapshots__/GenerateEventEmitterH-test.js.snap

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -390,6 +390,33 @@ namespace react {
390390
391391
392392
393+
} // namespace react
394+
} // namespace facebook
395+
",
396+
}
397+
`;
398+
399+
exports[`GenerateEventEmitterH can generate fixture EXCLUDE_IOS_TWO_COMPONENTS_DIFFERENT_FILES 1`] = `
400+
Map {
401+
"EventEmitters.h" => "
402+
/**
403+
* This code was generated by [react-native-codegen](https://www.npmjs.com/package/react-native-codegen).
404+
*
405+
* Do not edit this file as changes may cause incorrect behavior and will be lost
406+
* once the code is regenerated.
407+
*
408+
* @generated by codegen project: GenerateEventEmitterH.js
409+
*/
410+
#pragma once
411+
412+
#include <react/renderer/components/view/ViewEventEmitter.h>
413+
#include <jsi/jsi.h>
414+
415+
namespace facebook {
416+
namespace react {
417+
418+
419+
393420
} // namespace react
394421
} // namespace facebook
395422
",

packages/react-native-codegen/src/generators/components/__tests__/__snapshots__/GeneratePropsCpp-test.js.snap

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -412,6 +412,46 @@ ExcludedAndroidIosComponentProps::ExcludedAndroidIosComponentProps(
412412
}
413413
`;
414414
415+
exports[`GeneratePropsCpp can generate fixture EXCLUDE_IOS_TWO_COMPONENTS_DIFFERENT_FILES 1`] = `
416+
Map {
417+
"Props.cpp" => "
418+
/**
419+
* This code was generated by [react-native-codegen](https://www.npmjs.com/package/react-native-codegen).
420+
*
421+
* Do not edit this file as changes may cause incorrect behavior and will be lost
422+
* once the code is regenerated.
423+
*
424+
* @generated by codegen project: GeneratePropsCpp.js
425+
*/
426+
427+
#include <react/renderer/components/EXCLUDE_IOS_TWO_COMPONENTS_DIFFERENT_FILES/Props.h>
428+
#include <react/renderer/core/PropsParserContext.h>
429+
#include <react/renderer/core/propsConversions.h>
430+
431+
namespace facebook {
432+
namespace react {
433+
434+
ExcludedIosComponentProps::ExcludedIosComponentProps(
435+
const PropsParserContext &context,
436+
const ExcludedIosComponentProps &sourceProps,
437+
const RawProps &rawProps): ViewProps(context, sourceProps, rawProps)
438+
439+
440+
{}
441+
MultiFileIncludedNativeComponentProps::MultiFileIncludedNativeComponentProps(
442+
const PropsParserContext &context,
443+
const MultiFileIncludedNativeComponentProps &sourceProps,
444+
const RawProps &rawProps): ViewProps(context, sourceProps, rawProps),
445+
446+
disabled(convertRawProp(context, rawProps, \\"disabled\\", sourceProps.disabled, {true}))
447+
{}
448+
449+
} // namespace react
450+
} // namespace facebook
451+
",
452+
}
453+
`;
454+
415455
exports[`GeneratePropsCpp can generate fixture FLOAT_PROPS 1`] = `
416456
Map {
417457
"Props.cpp" => "

packages/react-native-codegen/src/generators/components/__tests__/__snapshots__/GeneratePropsH-test.js.snap

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -660,6 +660,52 @@ class JSI_EXPORT ExcludedAndroidIosComponentProps final : public ViewProps {
660660
}
661661
`;
662662
663+
exports[`GeneratePropsH can generate fixture EXCLUDE_IOS_TWO_COMPONENTS_DIFFERENT_FILES 1`] = `
664+
Map {
665+
"Props.h" => "
666+
/**
667+
* This code was generated by [react-native-codegen](https://www.npmjs.com/package/react-native-codegen).
668+
*
669+
* Do not edit this file as changes may cause incorrect behavior and will be lost
670+
* once the code is regenerated.
671+
*
672+
* @generated by codegen project: GeneratePropsH.js
673+
*/
674+
#pragma once
675+
676+
#include <jsi/jsi.h>
677+
#include <react/renderer/components/view/ViewProps.h>
678+
#include <react/renderer/core/PropsParserContext.h>
679+
680+
namespace facebook {
681+
namespace react {
682+
683+
class JSI_EXPORT ExcludedIosComponentProps final : public ViewProps {
684+
public:
685+
ExcludedIosComponentProps() = default;
686+
ExcludedIosComponentProps(const PropsParserContext& context, const ExcludedIosComponentProps &sourceProps, const RawProps &rawProps);
687+
688+
#pragma mark - Props
689+
690+
691+
};
692+
693+
class JSI_EXPORT MultiFileIncludedNativeComponentProps final : public ViewProps {
694+
public:
695+
MultiFileIncludedNativeComponentProps() = default;
696+
MultiFileIncludedNativeComponentProps(const PropsParserContext& context, const MultiFileIncludedNativeComponentProps &sourceProps, const RawProps &rawProps);
697+
698+
#pragma mark - Props
699+
700+
bool disabled{true};
701+
};
702+
703+
} // namespace react
704+
} // namespace facebook
705+
",
706+
}
707+
`;
708+
663709
exports[`GeneratePropsH can generate fixture FLOAT_PROPS 1`] = `
664710
Map {
665711
"Props.h" => "

packages/react-native-codegen/src/generators/components/__tests__/__snapshots__/GeneratePropsJavaDelegate-test.js.snap

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -436,6 +436,69 @@ exports[`GeneratePropsJavaDelegate can generate fixture EXCLUDE_ANDROID 1`] = `M
436436
437437
exports[`GeneratePropsJavaDelegate can generate fixture EXCLUDE_ANDROID_IOS 1`] = `Map {}`;
438438
439+
exports[`GeneratePropsJavaDelegate can generate fixture EXCLUDE_IOS_TWO_COMPONENTS_DIFFERENT_FILES 1`] = `
440+
Map {
441+
"java/com/facebook/react/viewmanagers/ExcludedIosComponentManagerDelegate.java" => "/**
442+
* This code was generated by [react-native-codegen](https://www.npmjs.com/package/react-native-codegen).
443+
*
444+
* Do not edit this file as changes may cause incorrect behavior and will be lost
445+
* once the code is regenerated.
446+
*
447+
* @generated by codegen project: GeneratePropsJavaDelegate.js
448+
*/
449+
450+
package com.facebook.react.viewmanagers;
451+
452+
import android.view.View;
453+
import androidx.annotation.Nullable;
454+
import com.facebook.react.uimanager.BaseViewManagerDelegate;
455+
import com.facebook.react.uimanager.BaseViewManagerInterface;
456+
457+
public class ExcludedIosComponentManagerDelegate<T extends View, U extends BaseViewManagerInterface<T> & ExcludedIosComponentManagerInterface<T>> extends BaseViewManagerDelegate<T, U> {
458+
public ExcludedIosComponentManagerDelegate(U viewManager) {
459+
super(viewManager);
460+
}
461+
@Override
462+
public void setProperty(T view, String propName, @Nullable Object value) {
463+
super.setProperty(view, propName, value);
464+
}
465+
}
466+
",
467+
"java/com/facebook/react/viewmanagers/MultiFileIncludedNativeComponentManagerDelegate.java" => "/**
468+
* This code was generated by [react-native-codegen](https://www.npmjs.com/package/react-native-codegen).
469+
*
470+
* Do not edit this file as changes may cause incorrect behavior and will be lost
471+
* once the code is regenerated.
472+
*
473+
* @generated by codegen project: GeneratePropsJavaDelegate.js
474+
*/
475+
476+
package com.facebook.react.viewmanagers;
477+
478+
import android.view.View;
479+
import androidx.annotation.Nullable;
480+
import com.facebook.react.uimanager.BaseViewManagerDelegate;
481+
import com.facebook.react.uimanager.BaseViewManagerInterface;
482+
483+
public class MultiFileIncludedNativeComponentManagerDelegate<T extends View, U extends BaseViewManagerInterface<T> & MultiFileIncludedNativeComponentManagerInterface<T>> extends BaseViewManagerDelegate<T, U> {
484+
public MultiFileIncludedNativeComponentManagerDelegate(U viewManager) {
485+
super(viewManager);
486+
}
487+
@Override
488+
public void setProperty(T view, String propName, @Nullable Object value) {
489+
switch (propName) {
490+
case \\"disabled\\":
491+
mViewManager.setDisabled(view, value == null ? true : (boolean) value);
492+
break;
493+
default:
494+
super.setProperty(view, propName, value);
495+
}
496+
}
497+
}
498+
",
499+
}
500+
`;
501+
439502
exports[`GeneratePropsJavaDelegate can generate fixture FLOAT_PROPS 1`] = `
440503
Map {
441504
"java/com/facebook/react/viewmanagers/FloatPropNativeComponentManagerDelegate.java" => "/**

0 commit comments

Comments
 (0)