CPLAT-3873 Function Components #606

sydneyjodon-wk · 2020-07-08T21:36:14Z

Motivation

Now that we've migrated to the new boilerplate, we need to implement function components so they can be used in over_react.

Changes

Add function component supporting APIs (uiFunction, PropsFactory) (implemented in Greg's spike branch)
Update parsing logic to validate function components.
Update codegen for functional components.
- Generate props config for each factory.
Write tests
- component integration tests
- parsing tests (validate that the correct errors are raised by the builder when there is something wrong with the boilerplate)
Update documentation
- Show how to declare in README, and uiFunction
Add snippet

Release Notes

Review

See CONTRIBUTING.md for more details on review types (+1 / QA +1 / +10) and code review process.

Please review:

QA Checklist

Tests were updated and provide good coverage of the changeset and other affected code
Manual testing was performed if needed
- Steps from PR author:
  - Run pbr serve example and verify that function components work as expected on http://localhost:8080/builder/
  - Add code snippets to IDE and verify that they work as expected.
- Anything falling under manual testing criteria outlined in CONTRIBUTING.md

Merge Checklist

While we perform many automated checks before auto-merging, some manual checks are needed:

A Client Platform member has reviewed these changes
There are no unaddressed comments - this check can be automated if reviewers use the "Request Changes" feature
For release PRs - Version metadata in Rosie comment is correct

aviary3-wk · 2020-07-08T21:37:35Z

Security Insights

No security relevant content was detected by automated scans.

Action Items

Review PR for security impact; comment "security review required" if needed or unsure
Verify aviary.yaml coverage of security relevant code

Questions or Comments? Reach out on Slack: #support-infosec.

greglittlefield-wf

Sorry, I got a notification when this PR was opened and couldn't help but take a peek 😅

I have one question around the use of v5 and reusing existing grouping logic; let me know if you'd like to talk it over in person!

lib/src/builder/parsing/declarations_from_members.dart

# Conflicts: # snippets/snippets.json # snippets/snippets.xml

This reverts commit b19542c.

This reverts commit 27bc400.

aaronlademann-wf

Line 705 of doc/new_boilerplate_migration.md needs to be updated to remove the "coming soon" bit from the heading.

aaronlademann-wf · 2020-07-22T16:14:05Z

README.md

+    // Return the rendered component contents here.
+    // The `props` variable is typed; no need for string keys!


Can we return something here to demonstrate things like prop forwarding, etc.?

doc/new_boilerplate_migration.md

example/builder/src/function_component.dart

aaronlademann-wf · 2020-07-22T16:24:18Z

example/builder/src/function_component.dart

+  }
+  final genericFactory = uiFunction<UiProps>(GenericFactory, null);
+
+  final basicFactory = uiFunction<BasicProps>(


IMO we should also have one utilizing forwardRef.

aaronlademann-wf · 2020-07-22T16:25:42Z

lib/src/builder/codegen/typed_map_impl_generator.dart

@@ -46,7 +46,7 @@ abstract class TypedMapImplGenerator extends BoilerplateDeclarationGenerator {

  TypedMapNames get names;
  bool get isComponent2;
-  FactoryNames get factoryNames;
+  List<FactoryNames> get factoryNames;


What is the tldr; on this change?

We need to generate a props config for every function component factory (even if they share the same props mixin) so that the display name is set up correctly. This has to be a list in order to get all the factory names from PropsMapViewOrFunctionComponentDeclaration.factories. In all other cases factoryNames would have a single element -- I just added an assert for that in _generateFactory.

lib/src/component_declaration/function_component.dart

pubspec.yaml

greglittlefield-wf

Some comments around generation, but this looks awesome!!

README.md

lib/src/builder/codegen/typed_map_impl_generator.dart

lib/src/builder/parsing/declarations_from_members.dart

lib/src/component_declaration/function_component.dart

lib/src/builder/parsing/declarations_from_members.dart

test/vm_tests/builder/declaration_parsing_test.dart

lib/src/component_declaration/function_component.dart

…ponents # Conflicts: # pubspec.yaml

Addressed feedback

sydneyjodon-wk

Hey @aaronlademann-wf @greglittlefield-wf I think I addressed all your feedback except that one discussion on FunctionComponentConfig which I put in slack. So this is ready for another round of review when you have time!

lib/src/builder/parsing/declarations_from_members.dart

lib/src/builder/parsing/members/factory.dart

lib/src/builder/parsing/ast_util.dart

lib/src/builder/codegen.dart

greglittlefield-wf

Just a couple minor things; this is looking awesome! I'll try and start QAing soon

lib/src/component_declaration/function_component.dart

greglittlefield-wf · 2020-07-27T20:18:03Z

lib/src/component_declaration/function_component.dart

+}
+
+/// Helper class used to keep track of generated information for [uiFunction].
+@protected


Now that we accept it as an argument to uiFunction, this class and its members shouldn't be @protected anymore

example/builder/src/function_component.dart

doc/new_boilerplate_migration.md

greglittlefield-wf

+1

QAing now...

greglittlefield-wf

Test coverage looks good
Examples look good and render as expected
Builder changes don't affect existing code

+10 🎉

sydneyjodon-wk added 3 commits July 7, 2020 10:31

Initial spike for other branch

dd8bb52

Add v5_functionComponent to parsing versions

a81cdf8

Update generated file and error handling

8e2d970

rmconsole-wf added the Merge Requirements Unmet label Jul 8, 2020

Parse ComponentFactoryDeclarations

d773544

greglittlefield-wf reviewed Jul 9, 2020

View reviewed changes

lib/src/builder/parsing/declarations_from_members.dart Outdated Show resolved Hide resolved

sydneyjodon-wk added 9 commits July 9, 2020 16:45

Remove v5 and update PropsMapViewOrFunctionComponentDeclaration

86bc327

Update example

6958e22

Add codegen for FunctionComponentConfig

7dc77f6

Fix test failures and format

027cd66

Update getPropsNameFromConfig

3e62161

Add initial parsing tests

e2da213

Generate config for each factory to keep correct displayName

42005e2

Add parsing for generic function components

685e9d7

Move defaultProps to react-dart

659692e

sydneyjodon-wk mentioned this pull request Jul 14, 2020

Function Component defaultProps Updates Workiva/react-dart#259

Closed

sydneyjodon-wk added 6 commits July 14, 2020 12:07

Update pubspec dependency override

1d614a9

Add codegen tests

f57ea6c

Handle parsing null config and custom props factory

68db3d4

Add function component snippets

d9d0708

Format

d71da40

Merge branch 'master' into CPLAT-3873-function-components

4885e6b

# Conflicts: # snippets/snippets.json # snippets/snippets.xml

rmconsole-wf added the Skynet 2.0 Failed label Jul 16, 2020

sydneyjodon-wk added 6 commits July 17, 2020 09:32

Initial integration tests

b19542c

Merge branch 'master' into CPLAT-3873-function-components

49d1f5e

Revert "Initial integration tests"

27bc400

This reverts commit b19542c.

Revert "Revert "Initial integration tests""

3828257

This reverts commit 27bc400.

Remove unused import

6fa267e

Add tests based on component tests

0a4d434

sydneyjodon-wk marked this pull request as ready for review July 22, 2020 15:51

aaronlademann-wf added area-builder area-component_base State: needs review Type: new feature labels Jul 22, 2020

aaronlademann-wf previously requested changes Jul 22, 2020

View reviewed changes

greglittlefield-wf previously requested changes Jul 22, 2020

View reviewed changes

greglittlefield-wf reviewed Jul 22, 2020

View reviewed changes

lib/src/component_declaration/function_component.dart Outdated Show resolved Hide resolved

Update based on reviewer feedback

cec79b9

aaronlademann-wf changed the base branch from master to function-components-wip July 23, 2020 15:15

aaronlademann-wf and others added 5 commits July 23, 2020 08:16

Merge 'upstream/function-components-wip' into CPLAT-3873-function-com…

f117e9d

…ponents # Conflicts: # pubspec.yaml

Remove yield for non-generated factories

d1c01d7

Add factory grouping and update examples

eaa69ca

Add parsing for hocs and update wrapper function typing

0e38d4d

Run build and add null check

82f92a4

sydneyjodon-wk commented Jul 24, 2020

View reviewed changes

greglittlefield-wf requested changes Jul 24, 2020

View reviewed changes

sydneyjodon-wk added 2 commits July 24, 2020 15:36

Update generic args for uiFunction

0fd444a

Update uiFunction args

e7a263d

greglittlefield-wf requested changes Jul 27, 2020

View reviewed changes

Address feedback

c22f0a2

greglittlefield-wf approved these changes Jul 27, 2020

View reviewed changes

rmconsole-wf added Merge Requirements Met and removed Merge Requirements Unmet labels Jul 27, 2020

greglittlefield-wf merged commit 62e05d8 into function-components-wip Jul 27, 2020

aaronlademann-wf mentioned this pull request Aug 7, 2020

RM-79371 Release over_react 3.8.0 #616

Merged

sydneyjodon-wk deleted the CPLAT-3873-function-components branch January 28, 2022 16:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CPLAT-3873 Function Components #606

CPLAT-3873 Function Components #606

sydneyjodon-wk commented Jul 8, 2020 •

edited by aaronlademann-wf

Loading

aviary3-wk commented Jul 8, 2020

greglittlefield-wf left a comment

aaronlademann-wf left a comment

aaronlademann-wf Jul 22, 2020

aaronlademann-wf Jul 22, 2020

aaronlademann-wf Jul 22, 2020

sydneyjodon-wk Jul 22, 2020

greglittlefield-wf left a comment

sydneyjodon-wk left a comment

greglittlefield-wf left a comment

greglittlefield-wf Jul 27, 2020

greglittlefield-wf left a comment

greglittlefield-wf left a comment

		// Return the rendered component contents here.
		// The `props` variable is typed; no need for string keys!

CPLAT-3873 Function Components #606

CPLAT-3873 Function Components #606

Conversation

sydneyjodon-wk commented Jul 8, 2020 • edited by aaronlademann-wf Loading

Motivation

Changes

Release Notes

Review

QA Checklist

Merge Checklist

aviary3-wk commented Jul 8, 2020

Security Insights

Action Items

greglittlefield-wf left a comment

Choose a reason for hiding this comment

aaronlademann-wf left a comment

Choose a reason for hiding this comment

aaronlademann-wf Jul 22, 2020

Choose a reason for hiding this comment

aaronlademann-wf Jul 22, 2020

Choose a reason for hiding this comment

aaronlademann-wf Jul 22, 2020

Choose a reason for hiding this comment

sydneyjodon-wk Jul 22, 2020

Choose a reason for hiding this comment

greglittlefield-wf left a comment

Choose a reason for hiding this comment

sydneyjodon-wk left a comment

Choose a reason for hiding this comment

greglittlefield-wf left a comment

Choose a reason for hiding this comment

greglittlefield-wf Jul 27, 2020

Choose a reason for hiding this comment

greglittlefield-wf left a comment

Choose a reason for hiding this comment

greglittlefield-wf left a comment

Choose a reason for hiding this comment

sydneyjodon-wk commented Jul 8, 2020 •

edited by aaronlademann-wf

Loading