Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CPLAT-7226 Deprecate setClientConfiguration #253

Merged

Conversation

joebingham-wk
Copy link
Collaborator

@joebingham-wk joebingham-wk commented Jan 28, 2020

Motivation

setClientConfiguration is unnecessary and the default configuration can be initialized automatically.

Changes

  • Add deprecations

    • setClientConfiguration
    • setReactConfiguration
    • setReactDOMConfiguration
    • setReactDOMServerConfiguration
  • Move APIs out of react_client.dart

    • Proxies (now a public library)
    • Statics (now a private file)
    • Event Factories (now a private file)
    • Registration Functions (registerComponent, registerComponent2, registerFunctionComponent - now a private file)
    • Utilities (there are both public and private utilities)
    • Zones (now a public library)
    • Typedefs (basically just the function component typedef. Moved into typedefs.dart and shown in react_client).
  • Expose the moved APIs from react_client.dart

  • Remove all mentions of setConfigurationClient

Review

Please review @aaronlademann-wf @greglittlefield-wf @sydneyjodon-wk

@joebingham-wk joebingham-wk marked this pull request as ready for review January 28, 2020 18:41
@semveraudit-wf
Copy link

semveraudit-wf commented Jan 28, 2020

Public API Changes

Recommendation: ‼️ Major version bump (fyi @Workiva/semver-audit-group )

@@ line 31: package:react/react_client/react_interop.dart @@
class React {}
+     void useImperativeHandle(JsRef ref, dynamic Function() createHandle, [List<dynamic> dependencies]);
//    Adding abstract members breaks all subclasses.
+ react/react_client/zone.dart

// Adding an entry point is a minor change.
@@ line 52: package:react/react_client/component_factory.dart @@
+  Map<dynamic, dynamic> unconvertJsProps(instance);
// Adding a top-level function is a minor change.
+ react/react_client/component_factory.dart

// Adding an entry point is a minor change.
@@ line 38: package:react/react_client/component_factory.dart @@
+  Function unconvertJsEventHandler(Function jsConvertedEventHandler);
// Adding a top-level function is a minor change.
Click to see 10 more API Changes


@@ line 476: package:react/hooks.dart @@
+  void useImperativeHandle(Ref ref, dynamic Function() createHandle, [List<dynamic> dependencies]);
// Adding a top-level function is a minor change.
@@ line 20: package:react/react_client/component_factory.dart @@
+  dynamic listifyChildren(dynamic children);
// Adding a top-level function is a minor change.
@@ line 87: package:react/react_client/component_factory.dart @@
+  class JsBackedMapComponentFactoryMixin {}
// Adding a class is a minor change.

+     ReactElement build(Map props, [List childrenArgs = const []]);
//    Adding a method is a minor change.

+     JsMap generateExtendedJsProps(Map props);
//    Adding a method is a minor change.

+     JsBackedMapComponentFactoryMixin();
//    Adding a constructor is a minor change.

      From package:react/react.dart
+     dynamic get type;
//    Adding a field is a minor change.
+     ReactJsComponentFactory reactComponentFactory;
//    Adding a field is a minor change.
+     dynamic call(Map props, [c1 = _notSpecified, c2 = _notSpecified, c3 = _notSpecified, c4 = _notSpecified, c5 = _notSpecified, c6 = _notSpecified, c7 = _notSpecified, c8 = _notSpecified, c9 = _notSpecified, c10 = _notSpecified, c11 = _notSpecified, c12 = _notSpecified, c13 = _notSpecified, c14 = _notSpecified, c15 = _notSpecified, c16 = _notSpecified, c17 = _notSpecified, c18 = _notSpecified, c19 = _notSpecified, c20 = _notSpecified, c21 = _notSpecified, c22 = _notSpecified, c23 = _notSpecified, c24 = _notSpecified, c25 = _notSpecified, c26 = _notSpecified, c27 = _notSpecified, c28 = _notSpecified, c29 = _notSpecified, c30 = _notSpecified, c31 = _notSpecified, c32 = _notSpecified, c33 = _notSpecified, c34 = _notSpecified, c35 = _notSpecified, c36 = _notSpecified, c37 = _notSpecified, c38 = _notSpecified, c39 = _notSpecified, c40 = _notSpecified]);
//    Adding a method is a minor change.
@@ line 264: package:react/react_client/component_factory.dart @@
+  class ReactJsComponentFactoryProxy extends ReactComponentFactoryProxy {}
// Adding a class is a minor change.

+     ReactJsComponentFactoryProxy(ReactClass jsClass, {this.shouldConvertDomProps: true, this.alwaysReturnChildrenAsList: false});
//    Adding a constructor is a minor change.

+     Function get factory;
//    Adding a field is a minor change.

+     bool get alwaysReturnChildrenAsList;
//    Adding a field is a minor change.

+     bool get shouldConvertDomProps;
//    Adding a field is a minor change.

+     ReactElement build(Map props, [List childrenArgs]);
//    Adding a method is a minor change.

      From package:react/react.dart
+     ReactClass get type;
//    Adding a field is a minor change.
+     dynamic call(Map props, [c1 = _notSpecified, c2 = _notSpecified, c3 = _notSpecified, c4 = _notSpecified, c5 = _notSpecified, c6 = _notSpecified, c7 = _notSpecified, c8 = _notSpecified, c9 = _notSpecified, c10 = _notSpecified, c11 = _notSpecified, c12 = _notSpecified, c13 = _notSpecified, c14 = _notSpecified, c15 = _notSpecified, c16 = _notSpecified, c17 = _notSpecified, c18 = _notSpecified, c19 = _notSpecified, c20 = _notSpecified, c21 = _notSpecified, c22 = _notSpecified, c23 = _notSpecified, c24 = _notSpecified, c25 = _notSpecified, c26 = _notSpecified, c27 = _notSpecified, c28 = _notSpecified, c29 = _notSpecified, c30 = _notSpecified, c31 = _notSpecified, c32 = _notSpecified, c33 = _notSpecified, c34 = _notSpecified, c35 = _notSpecified, c36 = _notSpecified, c37 = _notSpecified, c38 = _notSpecified, c39 = _notSpecified, c40 = _notSpecified]);
//    Adding a method is a minor change.
+     ReactJsComponentFactory reactComponentFactory;
//    Adding a field is a minor change.
@@ line 304: package:react/react_client/component_factory.dart @@
+  class ReactDomComponentFactoryProxy extends ReactComponentFactoryProxy {}
// Adding a class is a minor change.

+     String get name;
//    Adding a field is a minor change.

+     void convertProps(Map props);
//    Adding a method is a minor change.

+     ReactElement build(Map props, [List childrenArgs = const []]);
//    Adding a method is a minor change.

+     ReactDomComponentFactoryProxy(name);
//    Adding a constructor is a minor change.

+     Function get factory;
//    Adding a field is a minor change.

      From package:react/react.dart
+     ReactJsComponentFactory reactComponentFactory;
//    Adding a field is a minor change.
+     dynamic call(Map props, [c1 = _notSpecified, c2 = _notSpecified, c3 = _notSpecified, c4 = _notSpecified, c5 = _notSpecified, c6 = _notSpecified, c7 = _notSpecified, c8 = _notSpecified, c9 = _notSpecified, c10 = _notSpecified, c11 = _notSpecified, c12 = _notSpecified, c13 = _notSpecified, c14 = _notSpecified, c15 = _notSpecified, c16 = _notSpecified, c17 = _notSpecified, c18 = _notSpecified, c19 = _notSpecified, c20 = _notSpecified, c21 = _notSpecified, c22 = _notSpecified, c23 = _notSpecified, c24 = _notSpecified, c25 = _notSpecified, c26 = _notSpecified, c27 = _notSpecified, c28 = _notSpecified, c29 = _notSpecified, c30 = _notSpecified, c31 = _notSpecified, c32 = _notSpecified, c33 = _notSpecified, c34 = _notSpecified, c35 = _notSpecified, c36 = _notSpecified, c37 = _notSpecified, c38 = _notSpecified, c39 = _notSpecified, c40 = _notSpecified]);
//    Adding a method is a minor change.
+     String get type;
//    Adding a field is a minor change.
@@ line 214: package:react/react_client/component_factory.dart @@
+  class ReactJsContextComponentFactoryProxy extends ReactJsComponentFactoryProxy {}
// Adding a class is a minor change.

+     ReactElement build(Map props, [List childrenArgs]);
//    Adding a method is a minor change.

+     Function get factory;
//    Adding a field is a minor change.

+     bool get shouldConvertDomProps;
//    Adding a field is a minor change.

+     ReactJsContextComponentFactoryProxy(ReactClass jsClass, {this.shouldConvertDomProps: true, this.isConsumer: false, this.isProvider: false});
//    Adding a constructor is a minor change.

+     ReactClass get type;
//    Adding a field is a minor change.

+     JsMap generateExtendedJsProps(Map props);
//    Adding a method is a minor change.

+     bool get alwaysReturnChildrenAsList;
//    Adding a field is a minor change.

+     bool get isConsumer;
//    Adding a field is a minor change.

+     bool get isProvider;
//    Adding a field is a minor change.

      From package:react/react.dart
+     dynamic call(Map props, [c1 = _notSpecified, c2 = _notSpecified, c3 = _notSpecified, c4 = _notSpecified, c5 = _notSpecified, c6 = _notSpecified, c7 = _notSpecified, c8 = _notSpecified, c9 = _notSpecified, c10 = _notSpecified, c11 = _notSpecified, c12 = _notSpecified, c13 = _notSpecified, c14 = _notSpecified, c15 = _notSpecified, c16 = _notSpecified, c17 = _notSpecified, c18 = _notSpecified, c19 = _notSpecified, c20 = _notSpecified, c21 = _notSpecified, c22 = _notSpecified, c23 = _notSpecified, c24 = _notSpecified, c25 = _notSpecified, c26 = _notSpecified, c27 = _notSpecified, c28 = _notSpecified, c29 = _notSpecified, c30 = _notSpecified, c31 = _notSpecified, c32 = _notSpecified, c33 = _notSpecified, c34 = _notSpecified, c35 = _notSpecified, c36 = _notSpecified, c37 = _notSpecified, c38 = _notSpecified, c39 = _notSpecified, c40 = _notSpecified]);
//    Adding a method is a minor change.
+     ReactJsComponentFactory reactComponentFactory;
//    Adding a field is a minor change.
@@ line 100: package:react/react_client/component_factory.dart @@
+  class ReactDartComponentFactoryProxy<TComponent extends Component> extends ReactComponentFactoryProxy {}
// Adding a class is a minor change.

+     ReactElement build(Map props, [List childrenArgs = const []]);
//    Adding a method is a minor change.

+     Map<dynamic, dynamic> get defaultProps;
//    Adding a field is a minor change.

+     ReactClass get reactClass;
//    Adding a field is a minor change.

+     InteropProps generateExtendedJsProps(Map props, dynamic children, {Map defaultProps});
//    Adding a method is a minor change.

+     ReactDartComponentFactoryProxy(ReactClass reactClass);
//    Adding a constructor is a minor change.

      From package:react/react.dart
+     ReactClass get type;
//    Adding a field is a minor change.
+     dynamic call(Map props, [c1 = _notSpecified, c2 = _notSpecified, c3 = _notSpecified, c4 = _notSpecified, c5 = _notSpecified, c6 = _notSpecified, c7 = _notSpecified, c8 = _notSpecified, c9 = _notSpecified, c10 = _notSpecified, c11 = _notSpecified, c12 = _notSpecified, c13 = _notSpecified, c14 = _notSpecified, c15 = _notSpecified, c16 = _notSpecified, c17 = _notSpecified, c18 = _notSpecified, c19 = _notSpecified, c20 = _notSpecified, c21 = _notSpecified, c22 = _notSpecified, c23 = _notSpecified, c24 = _notSpecified, c25 = _notSpecified, c26 = _notSpecified, c27 = _notSpecified, c28 = _notSpecified, c29 = _notSpecified, c30 = _notSpecified, c31 = _notSpecified, c32 = _notSpecified, c33 = _notSpecified, c34 = _notSpecified, c35 = _notSpecified, c36 = _notSpecified, c37 = _notSpecified, c38 = _notSpecified, c39 = _notSpecified, c40 = _notSpecified]);
//    Adding a method is a minor change.
+     ReactJsComponentFactory reactComponentFactory;
//    Adding a field is a minor change.
@@ line 188: package:react/react_client/component_factory.dart @@
+  class ReactDartComponentFactoryProxy2<TComponent extends Component2> extends ReactComponentFactoryProxy with JsBackedMapComponentFactoryMixin implements ReactDartComponentFactoryProxy<TComponent extends Component> {}
// Adding a class is a minor change.

+     ReactClass get type;
//    Adding a field is a minor change.

+     ReactElement build(Map props, [List childrenArgs = const []]);
//    Adding a method is a minor change.

+     Map<dynamic, dynamic> get defaultProps;
//    Adding a field is a minor change.

+     ReactClass get reactClass;
//    Adding a field is a minor change.

+     JsMap generateExtendedJsProps(Map props);
//    Adding a method is a minor change.

+     ReactDartComponentFactoryProxy2(ReactClass reactClass);
//    Adding a constructor is a minor change.

      From package:react/react.dart
+     ReactJsComponentFactory reactComponentFactory;
//    Adding a field is a minor change.
+     dynamic call(Map props, [c1 = _notSpecified, c2 = _notSpecified, c3 = _notSpecified, c4 = _notSpecified, c5 = _notSpecified, c6 = _notSpecified, c7 = _notSpecified, c8 = _notSpecified, c9 = _notSpecified, c10 = _notSpecified, c11 = _notSpecified, c12 = _notSpecified, c13 = _notSpecified, c14 = _notSpecified, c15 = _notSpecified, c16 = _notSpecified, c17 = _notSpecified, c18 = _notSpecified, c19 = _notSpecified, c20 = _notSpecified, c21 = _notSpecified, c22 = _notSpecified, c23 = _notSpecified, c24 = _notSpecified, c25 = _notSpecified, c26 = _notSpecified, c27 = _notSpecified, c28 = _notSpecified, c29 = _notSpecified, c30 = _notSpecified, c31 = _notSpecified, c32 = _notSpecified, c33 = _notSpecified, c34 = _notSpecified, c35 = _notSpecified, c36 = _notSpecified, c37 = _notSpecified, c38 = _notSpecified, c39 = _notSpecified, c40 = _notSpecified]);
//    Adding a method is a minor change.
@@ line 340: package:react/react_client/component_factory.dart @@
+  class ReactDartFunctionComponentFactoryProxy extends ReactComponentFactoryProxy with JsBackedMapComponentFactoryMixin {}
// Adding a class is a minor change.

+     String get displayName;
//    Adding a field is a minor change.

+     ReactDartFunctionComponentFactoryProxy(DartFunctionComponent dartFunctionComponent, {String displayName});
//    Adding a constructor is a minor change.

+     JsFunctionComponent get reactFunction;
//    Adding a field is a minor change.

+     ReactElement build(Map props, [List childrenArgs = const []]);
//    Adding a method is a minor change.

      From package:react/react.dart
+     ReactJsComponentFactory reactComponentFactory;
//    Adding a field is a minor change.
+     JsFunctionComponent get type;
//    Adding a field is a minor change.
+     dynamic call(Map props, [c1 = _notSpecified, c2 = _notSpecified, c3 = _notSpecified, c4 = _notSpecified, c5 = _notSpecified, c6 = _notSpecified, c7 = _notSpecified, c8 = _notSpecified, c9 = _notSpecified, c10 = _notSpecified, c11 = _notSpecified, c12 = _notSpecified, c13 = _notSpecified, c14 = _notSpecified, c15 = _notSpecified, c16 = _notSpecified, c17 = _notSpecified, c18 = _notSpecified, c19 = _notSpecified, c20 = _notSpecified, c21 = _notSpecified, c22 = _notSpecified, c23 = _notSpecified, c24 = _notSpecified, c25 = _notSpecified, c26 = _notSpecified, c27 = _notSpecified, c28 = _notSpecified, c29 = _notSpecified, c30 = _notSpecified, c31 = _notSpecified, c32 = _notSpecified, c33 = _notSpecified, c34 = _notSpecified, c35 = _notSpecified, c36 = _notSpecified, c37 = _notSpecified, c38 = _notSpecified, c39 = _notSpecified, c40 = _notSpecified]);
//    Adding a method is a minor change.

Ignored changes - @visibleForTesting members:

@@ line 20: package:react/react_client/zone.dart @@
+  Zone componentZone = Zone.root;
// Adding a top-level variable is a minor change.

Showing results for bfd44cd

Powered by semver-audit-service. Please report any problems by filing an issue.
Reported by the dart semver audit client 2.0.3
Browse public API.

Last edited UTC Feb 03 at 22:03:01

Copy link
Collaborator

@aaronlademann-wf aaronlademann-wf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly nits / naming stuff. Otherwise looks great!

lib/react_client/react_interop.dart Outdated Show resolved Hide resolved
lib/react_client/react_proxies.dart Outdated Show resolved Hide resolved
lib/react_client/react_zone.dart Outdated Show resolved Hide resolved
lib/react_dom.dart Outdated Show resolved Hide resolved
lib/react_dom_server.dart Outdated Show resolved Hide resolved
lib/src/react_client/synthetic_event_factories.dart Outdated Show resolved Hide resolved
lib/src/react_client/utils.dart Outdated Show resolved Hide resolved
test/factory/dom_factory_test.dart Outdated Show resolved Hide resolved
lib/react_client.dart Outdated Show resolved Hide resolved
lib/react.dart Outdated Show resolved Hide resolved
@aaronlademann-wf aaronlademann-wf added this to the 5.4.0 milestone Jan 29, 2020
Copy link
Collaborator

@aaronlademann-wf aaronlademann-wf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

Copy link
Collaborator

@greglittlefield-wf greglittlefield-wf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just nits and a couple comments around util organization, otherwise looks amazing! This really cleans up all of our files!

@@ -19,7 +18,6 @@ class SimpleComponent extends react.Component {

void main() {
print("What");
setClientConfiguration();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Omg being able to remove all of these is so nice!!

Copy link
Collaborator Author

@joebingham-wk joebingham-wk Feb 3, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯And removing the react_client import with it feels awesome haha

lib/react_client/utils.dart Outdated Show resolved Hide resolved
/// Currently only involves converting a top-level non-[List] [Iterable] to
/// a non-growable [List], but this may be updated in the future to support
/// advanced nesting and other kinds of children.
dynamic listifyChildren(dynamic children) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, since these primarily used by component_factory.dart, is there a reason these functions aren't included in that file instead of making new public entrypoint?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, I can group them together again. I thought it was nice to keep the little utilities in their own home to keep files like component_factory.dart as focused as possible, but I can combine things!

lib/react_dom_server.dart Outdated Show resolved Hide resolved
lib/src/react_client/private_utils.dart Outdated Show resolved Hide resolved
lib/src/react_client/private_utils.dart Outdated Show resolved Hide resolved
lib/src/react_client/private_utils.dart Outdated Show resolved Hide resolved
lib/src/react_client/private_utils.dart Outdated Show resolved Hide resolved
Copy link
Collaborator

@greglittlefield-wf greglittlefield-wf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@greglittlefield-wf
Copy link
Collaborator

Major version bump indicated by semver bot is a false positive

QA +1

@greglittlefield-wf greglittlefield-wf merged commit b2ca719 into 5.4.0-wip Feb 3, 2020
@greglittlefield-wf greglittlefield-wf deleted the CPLAT-7226-deprecate-setClientConfiguration-v2 branch February 16, 2022 21:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants