Skip to content

Using document.write #3

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

Closed
simevidas opened this issue Sep 8, 2016 · 9 comments
Closed

Using document.write #3

simevidas opened this issue Sep 8, 2016 · 9 comments

Comments

@simevidas
Copy link

simevidas commented Sep 8, 2016

This is a comment to http://pomax.github.io/1473270609919/if-you-use-use-document-write-you-suck-at-javascript


I have this HTML code at the bottom of my site:

<script src="/js/ext/jquery.min.js"></script>
<script>
  window.Promise || document.write('<script src="/js/ext/promise.min.js"><\x3C/script>')
</script>
<script src="/js/ext/picker.min.js"></script>
<script src="/js/ext/marked.min.js"></script>
<script src="/js/site-client.min.js"></script>

So, I use document.write to synchronously insert a Promises polyfill only when the browser doesn’t support them natively (IE, etc.). What do you think of this pattern? Is it legit? 😋

@bevacqua
Copy link

bevacqua commented Sep 8, 2016

There are better solutions that don't involve document.write. Including a Promise polyfill isn't that expensive, so it wouldn't be very harmful to include it every time. There are better opportunities to optimize here, such as bundling and the async attribute.

@bkardell
Copy link

bkardell commented Sep 8, 2016

I've done that. Seems legit. I don't do it anymore but whatevs.

@felquis
Copy link

felquis commented Sep 8, 2016

Interesting article update here -> https://developers.google.com/web/updates/2016/08/removing-document-write

@simevidas
Copy link
Author

simevidas commented Sep 8, 2016

@felquis Note that this intervention only affects cross-origin scripts. Also, the document.write in my code is only evaluated in browsers that don’t support promises (e.g. IE), so modern browsers just skip over it.

@paulirish
Copy link

@simevidas the intervention is currently on cross-origin scripts. But I can imagine in 2017 we expand and do an intervention on same-origin ones as well.

script tags kinda suck, especially if they are under your control. if they're definitely under your control, then they should all have [defer] on them, and in that case, I'd go with something better for dynamically requiring a polyfill. Although admittedly there isn't an elegant solution.

I would disagree with @bevacqua though. IMO polyfills should only be served if its neccessary. Ideally this means the bundles you ship down are customized to the UA.

@bkardell
Copy link

bkardell commented Sep 8, 2016

@paulirish I get it, but I would be lying if I said this intervention did not make me a little uneasy already and the idea of continuing to push it further more so. I'm sure there have been discussions involving my concerns already, and I'd probably feel better if I read through them but... I'm not sure where to find them? (Looking through the links from above announcement I see very little discussion about what breaks and how we know and what that means).

@bevacqua
Copy link

bevacqua commented Sep 8, 2016

@paulirish Absolutely, but I'd rather use the async attribute and avoid document.write. Later, I can figure out how to load only polyfills I need and other micro-optimizations like that.

In this example, a Promise polyfill is way smaller than jQuery, marked, and probably the date picker, so if we're not worried about adding all of those, why would we care so much about the polyfill?

@hexalys
Copy link

hexalys commented Sep 9, 2016

@bkardell
I support the motive for long term. But not the approach taken. Which is not on point with the use case presented, and what this is supposed to solve. I have expressed my objection and constructive criticism.

PS: @simevidas
It explains why I recommended to you on Twitter to only use document.write() in the head.

@Pomax
Copy link
Owner

Pomax commented Feb 9, 2017

To comment on the original post (which I completely missed because for some reason github had not set me to watching my own repo... for months apparently):

@simevidas no that seems pretty silly to me =) Shims like these are tiny, and are typically written in a way that makes sure they don't shim anything when the browser they're loaded in already supports the feature they're shimming. So in edge or chrome or firefox the shim will load, do nothing, and the browser moves on to the next shim. No need for a document.write there. Does that possibly load an unnecessary file? Absolutely. But, unless you heavily optimized your page already for first-second page load and bandwidth-over-time (and it certainly doesn't look like that in this case) one more tiny shim is not going to make any appreciable difference. If you are optimizing, then optimizing for a UA in that UA feels like you're acting way too late: have the server remove any shims that won't be necessary based on a knowledge of your audience: users of older IE are not going to be using a custom UA to lie about which browser they're in, so have your server check who's asking for the data and prune your shim list accordingly, then make sure the shims that survive that process are bundled with your other scripts into a fast-first-presentation chunk and a can-be-deferred chunk, so the client only needs three http requests to get your (minified and served with gzip) HTML source plus associated (also minified and served with gzip) scripts.

That said, the only reasonable place where you might conceivable stick one and get away with "yeah I know how document.write works, which is why it's here" is in the <head> and then as a straight up script, as part of a codepath that is guaranteed to run synchronously. But even with that caveat I'll tell you to get rid of it and instead use something more modern.

@Pomax Pomax closed this as completed Apr 8, 2025
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

No branches or pull requests

7 participants