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 broken Glimmer component detection #697

Merged
merged 2 commits into from
Jan 12, 2021

Conversation

andrewpye
Copy link
Member

@andrewpye andrewpye commented Jan 12, 2021

At some point, component definitions in production switched to using Symbol keys instead of string keys. This broke our is-component-definition helper, which resulted in some breakages elsewhere. This PR adds a rudimentary check for these new-style component definitions.

This is at least the second time that this helper has broken as a result of Ember/Glimmer updates. I'm aware that we should be trying to figure out a way to remove it or make it less brittle/dependent on internals, but for the time being I think this workaround will have to do 😅

Demo of something now working after this change that was previously broken (the strong part of the help text was previously not updating because it was not receiving updates due to is-component-definition returning a false negative):
prod-1868-fix-component-definition-detection

@andrewpye andrewpye self-assigned this Jan 12, 2021
Copy link
Member

@vladucu vladucu left a comment

Choose a reason for hiding this comment

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

Yeah, would be good to get rid of these hacks.

Anyway, great work here man! This will hopefully do it for now 🥇

@vladucu vladucu added the bug Something isn't working label Jan 12, 2021
@andrewpye andrewpye merged commit a7c2478 into master Jan 12, 2021
@andrewpye andrewpye deleted the fix/broken-glimmer-component-detection branch January 12, 2021 12:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants