Skip to content

Conversation

maestromac
Copy link
Contributor

@maestromac maestromac commented Jun 9, 2019

What type of PR is this? (check all applicable)

  • Bug Fix

Description

It seems that having a standalone copy of webcomponent-loader.js is both insufficient and breaking JS on some browser, particularly WaterFox. WebComponent's suggestion to use polyfill is in the follow manner:

<script src="node_modules/@webcomponents/webcomponentsjs/webcomponents-loader.js"></script>

but that also is not a viable option since we can't load dependency from node-module like that. The next best thing we can do is asynchronously load the polyfill from the CDN based this instruction. The README stated that the loader is efficient and will load only what is necessary.

I have tried to not rely on CDN by letting webpack dynamically import it but that also doesn't seem to work without additional webpack configuration. If there's a more viable solution or I missed something, please let me know.

Related Tickets & Documents

#3081 and quite possibly some users on #2790

Mobile & Desktop Screenshots/Recordings (if there are UI changes)

n/a

Added to documentation?

  • no documentation needed

@pr-triage pr-triage bot added the PR: unreviewed bot applied label for PR's with no review label Jun 9, 2019
@@ -1 +1,10 @@
import 'clipboard-copy-element';
document.addEventListener('DOMContentLoaded', () => {
const waitingForOnboarding = setInterval(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

question: why did you wrap it in a set interval? doesn't it already "wait" ? I'm looking at the example you mentioned here https://github.com/webcomponents/webcomponentsjs/#asynchronous

Copy link
Contributor Author

@maestromac maestromac Jun 9, 2019

Choose a reason for hiding this comment

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

Thanks for pointing that out. I've tried so many things but I forgot to try the most obvious one 😆. I updated it and it seems to be working too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I now remember why, Waterfox doesn't seem to acknowledge the waitfor method.

Copy link
Contributor

@benhalpern benhalpern left a comment

Choose a reason for hiding this comment

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

I’m gonna merge this for now with an eye towards revisiting. The defer helps ensure it shouldn’t cause problems. We’ll circle back to this with a permanent fix in future (and with a clearer picture about browser compatibility and what we support)

@pr-triage pr-triage bot added PR: reviewed-approved bot applied label for PR's where reviewer approves changes and removed PR: unreviewed bot applied label for PR's with no review labels Jun 10, 2019
@benhalpern benhalpern merged commit f62bc7f into forem:master Jun 10, 2019
@pr-triage pr-triage bot added PR: merged bot applied label for PR's that are merged and removed PR: reviewed-approved bot applied label for PR's where reviewer approves changes labels Jun 10, 2019
@maestromac maestromac deleted the mac/bug/webcomponent-loader branch June 10, 2019 15:30
@pr-triage pr-triage bot added PR: reviewed-approved bot applied label for PR's where reviewer approves changes and removed PR: merged bot applied label for PR's that are merged labels Jun 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: reviewed-approved bot applied label for PR's where reviewer approves changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants