Workiva / react-dart

Dart Bindings for React JS
BSD 2-Clause "Simplified" License
412 stars 67 forks source link

`registerComponent2` involves twice component instance creation #393

Open busslina opened 5 months ago

busslina commented 5 months ago

One instance is created implicit by registerComponent2 and another one explicit when using the registered builder (componentBuilder({})). This duplicity is breaking* a library which attempts to be built on top of react package.

Can anyone put some light here?

* Not sure, but at least suspiciously.

busslina commented 5 months ago

Here it is where that first instance is created (See componentInstance):

/// Creates and returns a new factory proxy from the provided [componentFactory]
/// which produces a new JS `ReactClass` component class.
ReactDartComponentFactoryProxy2 registerComponent2(
  ComponentFactory<Component2> componentFactory, {
  Iterable<String> skipMethods = const ['getDerivedStateFromError', 'componentDidCatch'],
  Component2BridgeFactory? bridgeFactory,
}) {
  var errorPrinted = false;
  try {
    bridgeFactory ??= Component2BridgeImpl.bridgeFactory;

    final componentInstance = componentFactory();
    final componentStatics = ComponentStatics2(
      componentFactory: componentFactory,
      instanceForStaticMethods: componentInstance,
      bridgeFactory: bridgeFactory,
    );
    final filteredSkipMethods = _filterSkipMethods(skipMethods);

    // Cache default props and store them on the ReactClass so they can be used
    // by ReactDartComponentFactoryProxy and externally.
    JsBackedMap defaultProps;
    try {
      defaultProps = JsBackedMap.from(componentInstance.defaultProps);
    } catch (e, stack) {
      print('Error when registering Component2 when getting defaultProps: $e\n$stack');
      errorPrinted = true;
      rethrow;
    }

    JsMap? jsPropTypes;
    try {
      // Access `componentInstance.propTypes` within an assert so they get tree-shaken out of dart2js builds.
      assert(() {
        jsPropTypes = bridgeFactory!(componentInstance).jsifyPropTypes(componentInstance, componentInstance.propTypes);
        return true;
      }());
    } catch (e, stack) {
      print('Error when registering Component2 when getting propTypes: $e\n$stack');
      errorPrinted = true;
      rethrow;
    }

    final jsConfig2 = JsComponentConfig2(
      defaultProps: defaultProps.jsObject,
      contextType: componentInstance.contextType?.jsThis,
      skipMethods: filteredSkipMethods,
      propTypes: jsPropTypes ?? JsBackedMap().jsObject,
    );

    final displayName = componentInstance.displayName;

    /// Create the JS `ReactClass` component class
    /// with custom JS lifecycle methods.
    final reactComponentClass =
        createReactDartComponentClass2(ReactDartInteropStatics2.staticsForJs, componentStatics, jsConfig2)
          // This is redundant since we also set `name` below, but some code may depend on reading displayName
          // so we'll leave this in place for now.
          ..displayName = displayName;

    // This is a work-around to display the correct name in error boundary component stacks
    // (and in the React DevTools if we weren't already setting displayName).
    defineProperty(reactComponentClass, 'name', JsPropertyDescriptor(value: displayName));

    // ignore: invalid_use_of_protected_member
    reactComponentClass.dartComponentVersion = ReactDartComponentVersion.component2;

    return ReactDartComponentFactoryProxy2(reactComponentClass);
  } catch (e, stack) {
    if (!errorPrinted) print('Error when registering Component2: $e\n$stack');
    rethrow;
  }
}
busslina commented 5 months ago

So that initializer instance which seems not intended to be used by the final user... Could be a way to detect from within the component whether it has been created in mode initializer or in mode normal/final user?

I can think on have an instance static counter... (ugly and repetitive) or to have a Map<Type, int> counter (ugly).

busslina commented 5 months ago

Anyway, it seems as non-optimal to create a component instance, just for internal initializing, that will not be used in the component tree.