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-4476: Create workaround for ddc bug related to uninitialized maps #252

Merged
merged 3 commits into from
Feb 28, 2019

Conversation

corwinsheahan-wf
Copy link
Contributor

@corwinsheahan-wf corwinsheahan-wf commented Feb 27, 2019

Ultimate problem:

Uninitialized key-value pairs in Maps are treated differently between dart2js compiled output and ddc compiled output. Details: dart-lang/sdk#36052

How it was fixed:

Add a null aware operator and explicitly return null in generated prop/state getters when the props or state map does not contain a value for the key for any prop/state member

Testing suggestions:

  • CI Passes (tests added)

Potential areas of regression:

None

@corwinsheahan-wf corwinsheahan-wf requested a review from a team as a code owner February 27, 2019 21:37
@aviary2-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.

Copy link
Contributor

@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

var customNamespaceProp;

@Accessor(keyNamespace: 'custom namespace~~', key: 'custom key!')
var customKeyAndNamespaceProp;
Copy link
Contributor

Choose a reason for hiding this comment

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

#nit most of these props are unused

Copy link
Contributor

Choose a reason for hiding this comment

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

(Same goes for the other file like this)

@greglittlefield-wf
Copy link
Contributor

+1

@corwinsheahan-wf
Copy link
Contributor Author

QA +1

  • CI Passes

Merging into master.
@Workiva/release-management-p

aaronlademann-wf added a commit that referenced this pull request Oct 4, 2019
+ This workaround is no longer necessary. It was fixed in Dart 2.2.1, and our pubspec forces at least 2.4.0
+ Regression tests were left intact to guard against any SDK regressions
aaronlademann-wf added a commit that referenced this pull request Oct 7, 2019
+ This workaround is no longer necessary. It was fixed in Dart 2.2.1, and our pubspec forces at least 2.4.0
+ Regression tests were left intact to guard against any SDK regressions
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.

7 participants