Skip to content

CPLAT-4843 Check for Legacy Lifecycle Methods in Component2 Declaration#301

Merged
rmconsole3-wf merged 10 commits into3.1.0-wipfrom
CPLAT-4843-fail-unsupported-lifecycle
May 30, 2019
Merged

CPLAT-4843 Check for Legacy Lifecycle Methods in Component2 Declaration#301
rmconsole3-wf merged 10 commits into3.1.0-wipfrom
CPLAT-4843-fail-unsupported-lifecycle

Conversation

@joebingham-wk
Copy link
Contributor

@joebingham-wk joebingham-wk commented May 29, 2019

Motivation

Because of the inheritance structure of Component2, it is possible for a consumer to utilize lifecycle methods that no longer exist within React 16. To prohibit this, a check was needed to stop the builder if a component inheriting from Component2 was calling one those methods.

Changes

  • Add a check in declaration_parsing.dart looking for the legacy lifecycle methods within Component2.
  • Fix ToggleButton because it was breaking the build process.
  • Add Tests

Release Notes

Review

See CONTRIBUTING.md for more details on review types (+1 / QA +1 / +10) and code review process.

Please review:
@aaronlademann-wf @greglittlefield-wf @kealjones-wk @sydneyjodon-wk

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:
      • Pull down branch and make sure the ToggleButton example still behaves as expected.
    • 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

@aviary3-wk
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.

@aaronlademann-wf aaronlademann-wf added this to the 3.1.0 milestone May 29, 2019
Copy link
Contributor

@aaronlademann-wf aaronlademann-wf left a comment

Choose a reason for hiding this comment

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

Just the one indentation thing, but I'm +10 otherwise.

final method = firstComponent2Member.getMethod(methodName);

if (method != null) {
error('''Error within ${firstComponent2Member.name.name}.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is my bad @joebingham-wk ... but with ''' comments, in order for the indentation of the source to not influence the output, you have to wrap the string block in unindent(), which you can import from import 'package:over_react/src/util/string_util.dart';.

Otherwise... the error output in console will look like this...

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No problem - I've worked with comment blocks being logged out before and should have caught that. Thank you for pointing it out again!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@aaronlademann-wf that's done! Also, I updated the error message to use method.name to keep the message cleaner and not include the method annotations / body. Let me know if the other way was better though

Copy link
Contributor

@aaronlademann-wf aaronlademann-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
Contributor

@Workiva/release-management-pp

@rmconsole3-wf rmconsole3-wf merged commit 945b0ad into 3.1.0-wip May 30, 2019
@rmconsole3-wf rmconsole3-wf deleted the CPLAT-4843-fail-unsupported-lifecycle branch May 30, 2019 19:16
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.

5 participants