Skip to content

CPLAT-12090 Only expect propType errors in DDC#104

Merged
rmconsole5-wk merged 16 commits into
masterfrom
proptypes-only-in-ddc
Sep 8, 2020
Merged

CPLAT-12090 Only expect propType errors in DDC#104
rmconsole5-wk merged 16 commits into
masterfrom
proptypes-only-in-ddc

Conversation

@aaronlademann-wf
Copy link
Copy Markdown
Contributor

An upcoming change to the react package will make it so that propTypes are not included in dart2js compiled output.

These changes make it so that the propTypes testing utilities never cause an expect to fail in dart2js by conditionally utilizing the anything Matcher when the runtime is dart2js. This is necessary in order to release the react-dart changes "safely" (e.g. without causing a bunch of suddenly failing dart2js tests for consumers).

We will need to coordinate the release of that change in react-dart so that we release these changes in over_react_test immediately after.

Also in this changeset is a new utility function - runningInDDC() - which has been found in a number of consumer repos (under a different name), so it made sense to export it from here with a new name to avoid conflicts.

@aviary2-wf
Copy link
Copy Markdown

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
Copy Markdown
Contributor

@joebingham-wk joebingham-wk left a comment

Choose a reason for hiding this comment

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

The changes look great! They're nice and simple, and runningInDDC is dope. I had a couple small comments about tests, but that's it!

Comment thread test/over_react_test/custom_matchers_test.dart Outdated
Comment thread test/over_react_test/custom_matchers_test.dart Outdated
Comment thread test/over_react_test/custom_matchers_test.dart Outdated
Comment thread test/over_react_test/custom_matchers_test.dart Outdated
Comment thread test/over_react_test/custom_matchers_test.dart Outdated
Comment thread test/over_react_test/custom_matchers_test.dart Outdated
+ Instead of pinning the Dart SDK version
Copy link
Copy Markdown
Contributor

@joebingham-wk joebingham-wk left a comment

Choose a reason for hiding this comment

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

+1! I know there's a dependent PR, so I can formally approve once that's merged and the pubspec is clean. All the changes here look good to me though!

@rmconsole3-wf rmconsole3-wf changed the title Only expect propType errors in DDC CPLAT-12090 Only expect propType errors in DDC Sep 3, 2020
Comment thread lib/src/over_react_test/dart_util.dart Outdated
@@ -0,0 +1,6 @@
/// Returns whether the current runtime is `dartdevc`.
bool runningInDDC() {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I wonder if we should just move this to a common location 🤷

Comment thread lib/src/over_react_test/dart_util.dart Outdated
Comment thread lib/src/over_react_test/custom_matchers.dart Outdated
Copy link
Copy Markdown
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.

+10

This reverts commit a3e29b2.
Copy link
Copy Markdown
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.

+10

Copy link
Copy Markdown
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.

+10

@aaronlademann-wf
Copy link
Copy Markdown
Contributor Author

@Workiva/release-management-pp

Copy link
Copy Markdown

@rmconsole-wf rmconsole-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 from RM

@rmconsole5-wk rmconsole5-wk merged commit bfcf3f2 into master Sep 8, 2020
@rmconsole5-wk rmconsole5-wk deleted the proptypes-only-in-ddc branch September 8, 2020 17:35
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