CPLAT-11722 Add uiForwardRef #612

joebingham-wk · 2020-07-29T20:30:26Z

Motivation

When planning function components, we missed a couple cases:

when you want to forward a ref to a function component
when you want to forward a ref using something than the existing component's props (way less common / important to solve)
Example: https://reactjs.org/docs/forwarding-refs.html#forwarding-refs-to-dom-components

Changes

API Changes:

Add uiForwardRef
Update ref example to include different syntax examples
Refactor forwardRef tests to be able to share test coverage with uiForwardRef
Add uiForwardRef tests

Builder Changes:

Rename FunctionComponentConfig to UiFactoryConfig
Rename / refactor isFunctionComponentFactory to shouldGenerateConfig
Create a getter on Identifier that can be used to check if the name is a factory type
Switch out a couple checks on the identifier name with checks for the presence of a generated UiFactoryConfig

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:
  - QA that the refs print correctly in the example. Note that the components do have code around logging, but that's to keep some consistency between React's docs and our examples. Because there aren't any props updated, there isn't any re-rendering and no reason for the components to log.
- 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

…ct into CPLAT-11722-add-uiForwardRef

aviary2-wf · 2020-07-29T20:32:12Z

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.

joebingham-wk · 2020-08-05T15:57:26Z

test/vm_tests/builder/codegen_test.dart

@@ -707,7 +707,7 @@ main() {
              (props) {
                return Dom.div()();
              }, 
-              $BarPropsMixinConfig, // ignore: undefined_identifier


This seems like it should have been $BarConfig from the get go - my parser change use the name to detect that it's a generated config and therefore enforces the naming pattern ${FactoryName}Config. Let me know if this doesn't work, though!

joebingham-wk · 2020-08-05T15:57:41Z

test/vm_tests/builder/declaration_parsing_test.dart

@@ -1665,7 +1665,7 @@ main() {
                  (props) {
                    return Dom.div()();
                  },
-                  \$FooConfig, // ignore: undefined_identifier


This seems like it should have been $BarConfig from the get go - my parser change use the name to detect that it's a generated config and therefore enforces the naming pattern ${FactoryName}Config. Let me know if this doesn't work, though!

joebingham-wk · 2020-08-05T16:50:44Z

tools/analyzer_plugin/lib/src/diagnostic/boilerplate_validator.dart

@@ -253,16 +253,6 @@ extension on SourceSpan {
  SourceRange asRangeOrEmpty() => this == null ? SourceRange.EMPTY : asRange();
 }

-extension<E> on Iterable<E> {
-  void forEachIndexed(void Function(E, int) callback) {


The new analyzer version made improvements to the analysis of extensions and caught this was unused

lib/src/builder/parsing/ast_util.dart

web/component2/src/demos/ref.dart

smaifullerton-wk · 2020-08-06T18:22:30Z

talked to Joe about some puzzling things I found while QAing and he's going to take a look -

greglittlefield-wf

Just formatting and one minor thing

web/component2/src/demos/ref.dart

greglittlefield-wf

+1

smaifullerton-wk · 2020-08-07T18:11:08Z

QA +1

tested functionality in demos/ref.dart:

button ref forwarded to function component HTML is printed when button is clicked
button ref forwarded to higher order component HTML is printed when button is clicked
input element ref from class wrapped w/ uiForwardRef using asForwardRefConfig input value is printed
input element ref from class wrapped w/ uiForwardRef and generated config using custom props input value & custom prop are printed
deprecated forwardRef examples function as expected

smaifullerton-wk · 2020-08-07T18:18:58Z

QA +1 still stands - latest commit is a wording change only

aaronlademann-wf

+1

@Workiva/release-management-pp

rmconsole-wf

+1 from RM

joebingham-wk added 3 commits July 29, 2020 11:21

Merge branch 'function-components-wip' of github.com:Workiva/over_rea…

a29da58

…ct into CPLAT-11722-add-uiForwardRef

Initial implementation

53fd919

Add first go at examples

968de33

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

aviary2-wf added the Security Review Required label Jul 29, 2020

Add extension method

c542033

rmconsole-wf added the Merge Conflicts label Jul 30, 2020

joebingham-wk added 5 commits August 3, 2020 10:05

Add tests

841d102

Format tests

117cadb

Merge in base

68f4700

Clean up

96ba4ff

Rename FunctionComponentConfig to UiFactoryConfig

de94787

rmconsole-wf removed the Merge Conflicts label Aug 3, 2020

joebingham-wk added 4 commits August 3, 2020 16:38

Check for generated config

ea10a41

Fix parsing

d435a1f

Fix up example

d865b24

Fix CI

378a58d

joebingham-wk commented Aug 5, 2020

View reviewed changes

joebingham-wk marked this pull request as ready for review August 5, 2020 15:58

joebingham-wk requested review from sydneyjodon-wk, aaronlademann-wf, greglittlefield-wf, smaifullerton-wk, corwinsheahan-wf and willdrach-wk August 5, 2020 16:07

Fix breakages from analyzer 0.39.16

7d288eb

joebingham-wk force-pushed the CPLAT-11722-add-uiForwardRef branch from b8f4969 to 7d288eb Compare August 5, 2020 16:49

joebingham-wk commented Aug 5, 2020

View reviewed changes

greglittlefield-wf requested changes Aug 6, 2020

View reviewed changes

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

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

web/component2/src/demos/ref.dart Outdated Show resolved Hide resolved

joebingham-wk added 2 commits August 6, 2020 12:42

Address parser feedback

e5001d2

Fix up example

8189f1c

aviary3-wk removed the Security Review Required label Aug 7, 2020

joebingham-wk added 3 commits August 7, 2020 07:57

Add integration tests

49df84c

Format

acf2582

Update doc comment

af9e2e4

greglittlefield-wf requested changes Aug 7, 2020

View reviewed changes

web/component2/src/demos/ref.dart Outdated Show resolved Hide resolved

web/component2/src/demos/ref.dart Outdated Show resolved Hide resolved

Address more feedback 🎉

31b3414

greglittlefield-wf approved these changes Aug 7, 2020

View reviewed changes

rmconsole-wf added Merge Requirements Met and removed Merge Requirements Unmet labels Aug 7, 2020

Add displayName to doc comment

b0f9877

rmconsole-wf added Merge Requirements Unmet and removed Merge Requirements Met labels Aug 7, 2020

Fix example title

d12bca5

aaronlademann-wf approved these changes Aug 7, 2020

View reviewed changes

rmconsole-wf added RM Ready Merge Requirements Met and removed Merge Requirements Unmet labels Aug 7, 2020

rmconsole-wf approved these changes Aug 7, 2020

View reviewed changes

rmconsole2-wf merged commit 4ff4b37 into function-components-wip Aug 7, 2020

rmconsole2-wf deleted the CPLAT-11722-add-uiForwardRef branch August 7, 2020 20:07

aaronlademann-wf mentioned this pull request Aug 7, 2020

RM-79371 Release over_react 3.8.0 #616

Merged

aaronlademann-wf added Type: deprecation Type: enhancement labels Aug 10, 2020

dave-doty mentioned this pull request Aug 10, 2020

change forwardRef to uiForwardRef UC-Davis-molecular-computing/scadnano#451

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CPLAT-11722 Add uiForwardRef #612

CPLAT-11722 Add uiForwardRef #612

joebingham-wk commented Jul 29, 2020 •

edited

Loading

aviary2-wf commented Jul 29, 2020 •

edited by aviary3-wk

Loading

joebingham-wk Aug 5, 2020

joebingham-wk Aug 5, 2020

joebingham-wk Aug 5, 2020

smaifullerton-wk commented Aug 6, 2020

greglittlefield-wf left a comment

greglittlefield-wf left a comment

smaifullerton-wk commented Aug 7, 2020 •

edited

Loading

smaifullerton-wk commented Aug 7, 2020

aaronlademann-wf left a comment

rmconsole-wf left a comment

CPLAT-11722 Add uiForwardRef #612

CPLAT-11722 Add uiForwardRef #612

Conversation

joebingham-wk commented Jul 29, 2020 • edited Loading

Motivation

Changes

Release Notes

Review

QA Checklist

Merge Checklist

aviary2-wf commented Jul 29, 2020 • edited by aviary3-wk Loading

Security Insights

Action Items

joebingham-wk Aug 5, 2020

Choose a reason for hiding this comment

joebingham-wk Aug 5, 2020

Choose a reason for hiding this comment

joebingham-wk Aug 5, 2020

Choose a reason for hiding this comment

smaifullerton-wk commented Aug 6, 2020

greglittlefield-wf left a comment

Choose a reason for hiding this comment

greglittlefield-wf left a comment

Choose a reason for hiding this comment

smaifullerton-wk commented Aug 7, 2020 • edited Loading

smaifullerton-wk commented Aug 7, 2020

aaronlademann-wf left a comment

Choose a reason for hiding this comment

rmconsole-wf left a comment

Choose a reason for hiding this comment

joebingham-wk commented Jul 29, 2020 •

edited

Loading

aviary2-wf commented Jul 29, 2020 •

edited by aviary3-wk

Loading

smaifullerton-wk commented Aug 7, 2020 •

edited

Loading