avoidwork / filesize.js

JavaScript library to generate a human readable String describing the file size
https://filesizejs.com
BSD 3-Clause "New" or "Revised" License
1.61k stars 97 forks source link

Library is broken in non-ES6 browsers #107

Closed th0r closed 4 years ago

th0r commented 4 years ago

All the versions >= 5.0 are broken in browsers that don't support ES6 because of this line: https://github.com/avoidwork/filesize.js/blob/3a088db57570ff352eb9a33ccdcb9d07c9fcb04d/tsconfig.json#L9

It may potentially break even evergreen browsers because target: "esnext" doesn't transpile ESNext code at all which means output may contain unstable ES features (stage-4) which are not yet supported by the browsers natively.

binarykitchen commented 4 years ago

Yeah, same here. "browser": "lib/filesize.js" should point to a ES5 compatible script.

th0r commented 4 years ago

@binarykitchen no, it contains function arguments with default values - it's ES6.

binarykitchen commented 4 years ago

Yep and that's breaking on IE11. Can't we transpile it to ES5?

avoidwork commented 4 years ago

Yeah, just don't use 5.0; anything it might use won't exist in browsers you shouldn't be supporting today.

binarykitchen commented 4 years ago

Why not package an ES5 version within and refer to it in package.json? Have it transpiled before publishing? Many good npm libraries do that.

avoidwork commented 4 years ago

that's the way i had it for years, and then someone opened a PR to update the toolchain and output, and it seems reasonable because you shouldn't be supporting es5 in 2019 https://www.zdnet.com/article/microsoft-security-chief-ie-is-not-a-browser-so-stop-using-it-as-your-default/

avoidwork commented 4 years ago

I believe the rationale of the other PR was that you'd webpack today if you needed an es5 output, because there's now an esm file.

binarykitchen commented 4 years ago

Well, webpack's or browserify's default config is NOT to transpile any node modules. Hence they should already come with ES5 entry points.

And IE11 still hasn't reached end of life yet, still about 3% use it.

avoidwork commented 4 years ago

The company that produces it has had their chief security officer publically state that you shouldn't use it.

I don't use webpack, so this isn't an issue from my pov.

th0r commented 4 years ago

@avoidwork it's a common practice in libraries world to transpire to es5 by default to support IE or transpire to ES6 and mention in the readme that if you still have to support IE then use v4.

Another thing is your TS target is ESNext and your build may potentially break even in the latest Chrome as I mentioned in the main post.

avoidwork commented 4 years ago

@th0r common is whatever you think is popular. I didn't make the change, so open an issue and flag the original author if you'd like to converse with the right person.

th0r commented 4 years ago

common is whatever you think is popular

All the libraries/frameworks I know provide es5-transpiled code in the main field of the package.json.

avoidwork commented 4 years ago

cool story. this one did as well until someone in the community removed it. ergo, your statement is just relative to yourself (not a bad thing, just stating a fact). you don't have to run 5.x

th0r commented 4 years ago

I didn't make the change, so open an issue and flag the original author

Hmm, here is the PR with the change: https://github.com/avoidwork/filesize.js/pull/104 And all the commits there are yours.

And here is another opened issue with the same problem: https://github.com/avoidwork/filesize.js/issues/109

avoidwork commented 4 years ago

oh right, it came in that way. same deal applies, it's just modernized the same way. if you want to change it, so you can get the form you want just open the PR.

avoidwork commented 4 years ago

for context, i don't care about supporting browsers stuck on es5 syntax, and i don't support/use any tooling that requires it. ergo, i don't care, but i won't block anyone returning it for whatever reason; i'd assume that'd be another major bump?

avoidwork commented 4 years ago

@th0r why aren't you able to follow this example? https://github.com/avoidwork/tiny-lru/pull/27#discussion_r328870077 there's a reference of es2020 I'm not grasping how 1 project rolls forward fine* and this one doesn't.

avoidwork commented 4 years ago

I've reverted & rolled forward with 6.0 'cause this isn't required from my pov and it's totally screwing you over, so lose/lose.