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: improve browser compatibility #165

Merged
merged 2 commits into from Aug 26, 2021
Merged

Conversation

xxgjzftd
Copy link
Contributor

some browser such as Weixin built-in browser don't support setting srcdoc property of iframe element.
so es-module-shims doesn't work well in this browser because the featureDetectionPromise never be resolved.
and there are many user of the Weixin Official Accounts which use this browser.
and i think document.write has better compatibility.

@guybedford
Copy link
Owner

Thanks, I don't see any issue with landing this as the approach.

Alternatively if we did need to conditionally do the write path in future, is there a way to determine if srcdoc can be set? Eg does it fail "srcdoc" in iframe or a similar test?

@xxgjzftd
Copy link
Contributor Author

Thank you for your reply~
I tried my best to find an elegant solution, but failed.
Below are my test in Weixin(Wechat) built-in browser in iphone(all models).
The srcdoc prop always exists in the iframe dom, no matter we check it before or after appending the iframe to document.body, and it even exists when we check it in the next event loop.
Then, I checked the iframe.outerHTML.Such as the srcdoc prop, the result always includes srcdoc="...".
Then, I tried to modify the parent's some prop in the srcdoc's script to show that the srcdoc works.But I found the code in srcdoc are excuted asynchronously.Therefore, there is no appropriate time to check if srcdoc works.
It seems that there is no way to make the code in srcdoc being excuted synchronously.I tried to find a way in mdn, stackoverflow and google etc, but failed too.
And I find the load event of iframe isn't triggered in Weixin browser when we set the srcdoc prop.In other browser or when we don't set the srcdoc prop, the load event is triggered normally.
That's all.

@xxgjzftd
Copy link
Contributor Author

In addition, there is no error too.So, we can't use try catch or onerror etc.

@guybedford guybedford merged commit 7567920 into guybedford:main Aug 26, 2021
@guybedford
Copy link
Owner

Ok thanks for confirming. I've merged with a comment, the next release should be next week.

@lewisl9029
Copy link
Contributor

I was looking at my lighthouse scores today and found this note about the usage of document.write:

image

Reading up on the details here, it seems like our use case here probably isn't a real performance concern since it doesn't involve any network requests that might block parsing for an unreasonable amount of time.

However, the red lighthouse warning and the associated score impact might still affect SEO (?) and is going to be hard to explain away to non-technical stakeholders, and can very easily drive people away from using es-module-shims.

I was wondering if maybe loading a blob URL in the iframe as src instead of setting srcdoc could be a reasonable alternative?

Happy to experiment with this and send a PR if that sounds like a good plan. Though I might need help from @xxgjzftd to verify this works in the Weixin browser (caniuse doesn't seem to have any data on it? https://caniuse.com/?search=createObjectURL).

@guybedford
Copy link
Owner

@lewisl9029 I'd also have preferred if we could have avoided document.write, although as seen in the discussion we couldn't find a way to detect support for srcdoc in weixin browser. I think feature detecting would be the better approach here, and one option could be to actually do an active feature detection like setting a srcdoc and then verifying if it has been applied by reading the HTML or some kind of check like that.

Such a test could be constructed without access to the weixin browser since it would just be positively verifying that srcdoc is applying in other browsers.

Let me know if that sort of approach might make sense? A PR would be great if you would like to work on this.

@lewisl9029
Copy link
Contributor

setting a srcdoc and then verifying if it has been applied by reading the HTML or some kind of check like that.

@guybedford from #165 (comment), it sounds like @xxgjzftd already tried something along those lines and couldn't distinguish between the Weixin Browser and others:

The srcdoc prop always exists in the iframe dom, no matter we check it before or after appending the iframe to document.body, and it even exists when we check it in the next event loop.
Then, I checked the iframe.outerHTML.Such as the srcdoc prop, the result always includes srcdoc="...".

The comment describes a bunch of other "active" feature detection approaches he attempted with little success, and seems pretty exhaustive to be honest, and I don't have any other ideas to try off the top of my head along those lines.

I was hoping we could maybe bypass the shakey srcdoc support situation entirely by using createObjectURL and setting src on the iframe, which I'm hoping will have better support. In fact es-module-shims already relies on createObjectURL heavily in its actual operation, so support is almost guaranteed in any browser where es-module-shim actually works as expected.

Curious if you're aware of any reasons why src & createObjectURL might not work for this purpose?

@lewisl9029
Copy link
Contributor

@xxgjzftd looks like the new fix was shipped in 1.3.3: https://github.com/guybedford/es-module-shims/releases/tag/1.3.3

When you have the chance, would you mind upgrading to 1.3.3 or later and verifying that this new version still works in Weixin browser? Thanks!

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.

None yet

3 participants