tbranyen/diffhtml

`new Set()` is not supported in old browsers

tiye opened this issue · 11 comments

tiye commented

We got reports that diffhtml throw errors in some mobile browsers since Set is now implemented. I thought it was compiled by Babel?

https://github.com/tbranyen/diffhtml/blob/master/dist/diffhtml.js#L2256
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Set#Browser_compatibility

jamesplease commented

Are you including the Babel polyfill?

tbranyen commented

I would include the polyfill in your app, and I consider it a feature that its not transpiled.

tiye commented

As usual we use diffhtml from npm and our Babel loader will not compile code inside node_modules/ folder. And it's "include the polyfill in our apps" in our side.

jamesplease commented

Ah, gotcha. So you need diffHTML to be transpired before distribution. Makes sense to me.

tiye commented

yeah, I want dist/diffhtml.js and other code in dist/ compiled before pushing to npm

tbranyen commented

By compiled, do you mean to ES5? I don't think I should add in a polyfill for Set though, because then I'd need to polyfill Promise, and Map. I might be able to publish a diffhtml.legacy.js file that contains the polyfills if you think that'd help?

tiye commented

I think it's more about the CommonJS code we talked about last time. And diffhtml.legacy.js looks good to me.

tbranyen commented

Yeah the CommonJS code is published to npm isn't it?

  • 👍1
jamesplease commented

I don't think that the polyfills should be managed in this repo. corejs has all of those covered pretty thoroughly...it'd be adding extra maintenance to this project for pretty much 0 gain for the consumer, I think. Because most folks transpile code, I would expect most people to already be using the polyfill.

My suggestion would be to:

  1. transpile the source through Babel, and distribute those as the main field in package.json
  2. also include the src files in the bundle in case someone wants to transpile the source themselves
  3. document the polyfills. Include the list of polyfills they need from corejs to run the lib (they can pull just those in from corejs already), or just say that babel's polyfill will have you covered.
  • 👍1
tbranyen commented

@jmeas

  • The main field is currently the built UMD file, which I find to be generally more useful than pointing to CJS transpiled files. Although the CJS files are present in the dist directory during npm publish.
  • The source is also included since many will want to use ES6 import/exports.
  • We'll have to do a quick audit for what polyfills will be necessary. Off the top of my head I can think of: Promise, Map, and Set, but there may be more like Array.from.
  • 👍1
tbranyen commented

Added a note in the README about polyfills here: https://github.com/tbranyen/diffhtml#polyfills

  • 👍2