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

Allow CustomElements to be redefined #197

Merged
merged 8 commits into from
Apr 30, 2022
Merged

Allow CustomElements to be redefined #197

merged 8 commits into from
Apr 30, 2022

Conversation

manuelpuyol
Copy link
Contributor

I want to allow CustomElements to be redefined when using an HMR plugin. Most plugins override the define method on customElements to not raise an error on redefinition.
Running the .define call in a try/catch will have the same behavior if no HMR plugin is present, but allow the plugin to run if it is

@manuelpuyol manuelpuyol requested a review from a team as a code owner April 4, 2022 16:33
Copy link
Contributor

@koddsson koddsson left a comment

Choose a reason for hiding this comment

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

Is it possible to do this only in development? I'm scared we might swallow legitimate errors in the future with the blanket try..catch.

Copy link
Member

@keithamus keithamus left a comment

Choose a reason for hiding this comment

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

This LGTM 👍

@keithamus keithamus requested a review from a team as a code owner April 30, 2022 07:20
@manuelpuyol manuelpuyol merged commit 0775b42 into main Apr 30, 2022
@manuelpuyol manuelpuyol deleted the allow-hmr-plugins branch April 30, 2022 07:36
keithamus added a commit to github/relative-time-element that referenced this pull request Nov 22, 2022
This moves the `customeElements.define` calls from each respective
element and into the index.js file. This is useful for if we wish to
load the components classes without defining the custom element.

In addition, this changes the pattern of how we register the custom
elements, to allow for HMR, as we did in
github/catalyst#197.
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