FED-279: Add improved prop forwarding method #829

kealjones-wk · 2023-07-19T22:12:51Z

Motivation

OverReact's prop forwarding story (fka "Consumed Props") for uiFunction (and UiComponent2) feels overly verbose and tedious. It also just doesn't really match the ease that comes with traditional React prop forwarding via destructuring and spread syntax in JSX, eg. ...rest.

Changes

Added two new extension methods to UiProps
- getPropsToForward for use with addAll (ex. ..addAll(props.getPropsToForward(exclude: {FooPropsMixin})))
- addPropsToForward for use with modifyProps (ex. ..modifyProps(props.addPropsToForward(exclude: {FooPropsMixin})))
  - exclude named argument will remove the props belonging to that Set of props mixins from the forwarded props.
  - domOnly named argument to forward only DOM based props.

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:
- 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 Frontend Frameworks Design 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

aviary-wf · 2023-07-19T22:14:11Z

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.

lib/src/component_declaration/builder_helpers.dart

greglittlefield-wf

Made some comments but generally this looks great to me!

@kealjones-wk Were you still planning on updating existing documentation on prop forwarding to recommend this new pattern? Also, would you making sure the PR description is up to date? (it still mentions include)

lib/src/component_declaration/builder_helpers.dart

...component_declaration/builder_integration_tests/new_boilerplate/function_component_test.dart

web/uifunction/src/utils.dart

web/uifunction/src/component_wrapper.dart

lib/src/component_declaration/builder_helpers.dart

kealjones-wk · 2023-08-10T19:50:41Z

doc/new_boilerplate_migration.md

@@ -23,18 +23,18 @@ _Preview of new boilerplate:_

 ## Background

-While converting the boilerplate for OverReact component declaration from Dart 1 


Sorry my IDE decided to "fix weird line terminations" or something?

apparently i cant even undo it now....

eyyy TextEdit to the rescue!

This reverts commit 5b1de45.

This reverts commit e43030d.

kealjones-wk · 2023-08-10T20:44:12Z

doc/new_boilerplate_migration.md

@@ -781,42 +781,117 @@ UiFactory<FooProps> Foo = uiFunction(
 ); 
 ```

-#### With Consumed Props
+#### With Prop Forwarding (fka Consumed Props)


Maybe this should go somewhere else? ugh i hate deciding where docs should go! @joebingham-wk i need your help lol

I think here and in the doc/props_mixin_component_composition.md section is sufficient for this PR 👍

greglittlefield-wf

Not sure if this is ready for another full round of review, but I skimmed the docs changes and they look good to me, and I spotted a couple other things

...component_declaration/builder_integration_tests/new_boilerplate/function_component_test.dart

lib/src/component_declaration/builder_helpers.dart

greglittlefield-wf

+10!

Gonna see if anyone else on the team wants to weigh in before we merge

...component_declaration/builder_integration_tests/new_boilerplate/function_component_test.dart

…s/new_boilerplate/function_component_test.dart Co-authored-by: Greg Littlefield <greg.littlefield@workiva.com>

- gosh darnit greg

greglittlefield-wf

+10 refresh

kealjones-wk · 2023-08-14T17:59:57Z

@Workiva/release-management-p

kealjones-wk · 2023-08-14T18:08:15Z

@Workiva/release-management-p

rmconsole-wf

+1 from RM

kealjones-wk and others added 3 commits August 1, 2022 16:07

spike for unused props concept

07c20b0

Merge branch 'master' into unused_props

e7c1eba

I heard you liked forwarding props...

69f836f

rmconsole-wf added the Merge Requirements Unmet label Jul 19, 2023

aviary-wf added the Security Review Required label Jul 19, 2023

greglittlefield-wf reviewed Jul 19, 2023

View reviewed changes

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

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

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

remove include

09cc932

rmconsole-wf added the Skynet 2.0 Failed label Jul 20, 2023

kealjones-wk added 5 commits July 21, 2023 14:14

add some dom tests

638aa33

Merge branch 'master' into unused_props

a1f18b9

uncomment old tests and fix generic test

37aae20

fix warnings

a6007d7

fix PropsToForward doc comment

4c7a63f

rmconsole-wf removed the Skynet 2.0 Failed label Jul 26, 2023

kealjones-wk added 5 commits July 26, 2023 15:01

formatting

98fe1d3

simplify consumedProps assignment in _propsToForward

4b76cc2

remove commented out code

2e30341

format

7b2799b

fix doc comments

8a5f7d8

kealjones-wk marked this pull request as ready for review July 26, 2023 22:29

kealjones-wk requested a review from greglittlefield-wf July 26, 2023 22:29

add prop forwarding demo to web

91a8572

rmconsole-wf added the Skynet 2.0 Failed label Jul 27, 2023

greglittlefield-wf requested changes Aug 2, 2023

View reviewed changes

kealjones-wk added 5 commits August 9, 2023 08:25

add test for error case

3c39d20

address feedback

fb07ac0

fix error test

31a9bad

add rethrow to propsToForward try catch

f8d89de

only run in ddc

80b2bab

aviary3-wk removed the Security Review Required label Aug 10, 2023

update docs?

e43030d

kealjones-wk commented Aug 10, 2023

View reviewed changes

kealjones-wk added 5 commits August 10, 2023 12:55

remove the line termination change

5b1de45

Revert "remove the line termination change"

0ad2e6d

This reverts commit 5b1de45.

Revert "update docs?"

d0b6d7d

This reverts commit e43030d.

add other doc updates

23c11e9

update migration guide?

eef837c

kealjones-wk commented Aug 10, 2023

View reviewed changes

another doc update

81010e5

greglittlefield-wf requested changes Aug 10, 2023

View reviewed changes

...component_declaration/builder_integration_tests/new_boilerplate/function_component_test.dart Outdated Show resolved Hide resolved

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

address feedback

6bfad61

kealjones-wk requested a review from greglittlefield-wf August 11, 2023 18:22

greglittlefield-wf previously approved these changes Aug 14, 2023

View reviewed changes

...component_declaration/builder_integration_tests/new_boilerplate/function_component_test.dart Outdated Show resolved Hide resolved

hunterbreathat-wk previously approved these changes Aug 14, 2023

View reviewed changes

kealjones-wk dismissed stale reviews from hunterbreathat-wk and greglittlefield-wf via cfd533e August 14, 2023 17:32

kealjones-wk and others added 2 commits August 14, 2023 10:32

Update test/over_react/component_declaration/builder_integration_test…

cfd533e

…s/new_boilerplate/function_component_test.dart Co-authored-by: Greg Littlefield <greg.littlefield@workiva.com>

Fix improper having use from suggestion

3a82784

- gosh darnit greg

kealjones-wk requested a review from greglittlefield-wf August 14, 2023 17:43

greglittlefield-wf approved these changes Aug 14, 2023

View reviewed changes

rmconsole-wf added the RM Ready label Aug 14, 2023

rmconsole-wf added Merge Requirements Met and removed Merge Requirements Unmet labels Aug 14, 2023

rmconsole-wf approved these changes Aug 14, 2023

View reviewed changes

rm-astro-wf merged commit 977b4d6 into master Aug 14, 2023
7 checks passed

rm-astro-wf deleted the unused_props branch August 14, 2023 18:18

rmconsole-readonly-wk mentioned this pull request Aug 14, 2023

RM-193073 RM-193072 Release over_react 4.10.0 #830

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FED-279: Add improved prop forwarding method #829

FED-279: Add improved prop forwarding method #829

kealjones-wk commented Jul 19, 2023 •

edited

Loading

aviary-wf commented Jul 19, 2023 •

edited by aviary3-wk

Loading

greglittlefield-wf left a comment

kealjones-wk Aug 10, 2023 •

edited

Loading

kealjones-wk Aug 10, 2023

kealjones-wk Aug 10, 2023

kealjones-wk Aug 10, 2023

greglittlefield-wf Aug 14, 2023

greglittlefield-wf left a comment

greglittlefield-wf left a comment

greglittlefield-wf left a comment

kealjones-wk commented Aug 14, 2023

kealjones-wk commented Aug 14, 2023

rmconsole-wf left a comment

		@@ -23,18 +23,18 @@ _Preview of new boilerplate:_

		## Background

		While converting the boilerplate for OverReact component declaration from Dart 1

FED-279: Add improved prop forwarding method #829

FED-279: Add improved prop forwarding method #829

Conversation

kealjones-wk commented Jul 19, 2023 • edited Loading

Motivation

Changes

Release Notes

Review

QA Checklist

Merge Checklist

aviary-wf commented Jul 19, 2023 • edited by aviary3-wk Loading

Security Insights

Action Items

greglittlefield-wf left a comment

Choose a reason for hiding this comment

kealjones-wk Aug 10, 2023 • edited Loading

Choose a reason for hiding this comment

kealjones-wk Aug 10, 2023

Choose a reason for hiding this comment

kealjones-wk Aug 10, 2023

Choose a reason for hiding this comment

kealjones-wk Aug 10, 2023

Choose a reason for hiding this comment

greglittlefield-wf Aug 14, 2023

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

greglittlefield-wf left a comment

Choose a reason for hiding this comment

kealjones-wk commented Aug 14, 2023

kealjones-wk commented Aug 14, 2023

rmconsole-wf left a comment

Choose a reason for hiding this comment

kealjones-wk commented Jul 19, 2023 •

edited

Loading

aviary-wf commented Jul 19, 2023 •

edited by aviary3-wk

Loading

kealjones-wk Aug 10, 2023 •

edited

Loading