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-4674: Add workaround for ddc bug via props/state impl constructor #256

Merged
merged 5 commits into from
Mar 4, 2019

Conversation

corwinsheahan-wf
Copy link
Contributor

@corwinsheahan-wf corwinsheahan-wf commented Mar 1, 2019

Ultimate problem:

There's an unknown ddc bug that affects private props/state members and classes. What we know so far: https://jira.atl.workiva.net/browse/CPLAT-4673

How it was fixed:

See https://jira.atl.workiva.net/browse/CPLAT-4673

Testing suggestions:

  • CI Passes
  • Verify tests pass in cplat-2734_dart2 branch of wsd on ddc with this branch publinked in.

Potential areas of regression:

Pretty much everything.

@aviary-wf
Copy link

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.

..writeln(' // This initializer of `_props` to an empty map, as well as the reassignment')
..writeln(' // of `_props` in the constructor body is necessary to work around an unknown ddc issue.')
..writeln(' // See <https://jira.atl.workiva.net/browse/CPLAT-4673> for more details')
..writeln(' $implName(Map backingMap) : this._props = {} {')
Copy link
Contributor

Choose a reason for hiding this comment

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

Just thought of one thing; have you tried initializing to null instead?

Would be nice to avoid this object creation if it's easy to; no big deal if it's a pain to try out, though.

Suggested change
..writeln(' $implName(Map backingMap) : this._props = {} {')
..writeln(' $implName(Map backingMap) : this._props = null {')

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately this doesn't work in ddc.

main() {
test('sets private props correctly in `getDefaultProps`', () {
var instance = render((Foo()
..stringProp = 'some string value'
Copy link
Contributor

Choose a reason for hiding this comment

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

#nit this prop is unused

@seanburke-wf
Copy link

QA +1

19:41 +26391 ~6: All tests passed!

@seanburke-wf
Copy link

@Workiva/release-management-p for merge into master

@rmconsole4-wk rmconsole4-wk merged commit 85ccb29 into master Mar 4, 2019
@rmconsole4-wk rmconsole4-wk deleted the add_workaround_unknown_ddc_bug branch March 4, 2019 22:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants