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

fix(wire): remove WireContextEvent (camelcase) #1681

Merged
merged 1 commit into from
Feb 11, 2020

Conversation

kevinv11n
Copy link
Contributor

Details

Followup to #1679 to remove legacy WireContextEvent. Use wirecontextevent instead.

Does this PR introduce breaking changes?

  • 🚨 Yes, it does introduce breaking changes.

Single consumer of WireContextEvent has been updated to use wirecontextevent so no impact expected.

@@ -253,7 +253,7 @@ export class WireEventTarget {
});
this._cmp.dispatchEvent(internalDomEvent);
return false; // canceling signal since we don't want this to propagate
} else if (evt.type === 'WireContextEvent' || evt.type === 'wirecontextevent') {
} else if (evt.type === 'wirecontextevent') {
Copy link
Contributor

Choose a reason for hiding this comment

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

are there tests for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

None that I found. It seems when the feature was implemented no tests were added.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Those tests exercise the v2 context mechanism: LinkContextEvent. I believe this WireContextEvent is the legacy v1 implementation which still has one consumer owned by @jbenallen .

Copy link
Contributor

Choose a reason for hiding this comment

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

got it.

@kevinv11n kevinv11n removed the nomerge label Feb 11, 2020
@kevinv11n kevinv11n merged commit 33b91a2 into master Feb 11, 2020
@kevinv11n kevinv11n deleted the kv/remove-WireContextEvent branch February 11, 2020 03:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants