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

[Discussion] Is an ES5 build/distribution necessary? #2642

Closed
puzrin opened this issue Jul 29, 2020 · 63 comments
Closed

[Discussion] Is an ES5 build/distribution necessary? #2642

puzrin opened this issue Jul 29, 2020 · 63 comments
Labels
package/build Issues relating to npm or packaging

Comments

@puzrin
Copy link

puzrin commented Jul 29, 2020

Editor: Adding some answers here at the tope for anyone else who finds this with Google.

If you're using the library as a direct dependency

There is no need for an ES5 build if you're using the library as a first-order dependency. Just build us separately and drop the web build of the library directly into your project. (either as a separate JS <script> tag) or concat it to the end of your monolithic build. Simply:

# from any unix shell
cat highlight.min.js your_monolithic_js_bundle.js > final_build.js

And of course you can do this easily with a build system as well.

If you're using Browserify

Evidently you can simply enable the global option. [more details would be helpful]


The original issue:

Ref: #2501

As you know, v10 announced stop support of IE11. But, indirectly, published releases become not es5-compatible any more. If you have cases where es5 (babelified src) is still needed (except IE11) - please post here details.

Support of es5 may be considered for prolong if a lot of demand exists.


[Maintainer]: I know it's mentioned above, but just to be very clear this issue is NOT about IE11 (or other legacy browsers) support. The ship has sailed on that. This is only about whether there is value in proving a pre-build ES5 distribution to use with picky/older build systems that haven't caught up to ES6 yet.

@puzrin puzrin added the enhancement An enhancement or new feature label Jul 29, 2020
@joshgoebel joshgoebel changed the title [Poll] Is es5 disribuion still required? [Discussion] Is an ES5 build/distribution necessary? Jul 29, 2020
@joshgoebel joshgoebel removed the enhancement An enhancement or new feature label Jul 29, 2020
@joshgoebel
Copy link
Member

Also, I'm very curious how others have solved this problem (if anyone has it) as I assume there are indeed ways for consumers of our NPM library to run babel against it themselves during their build process and ES5-ify it. I'm not sure how hard or easy that might be...

@joshgoebel joshgoebel added the package/build Issues relating to npm or packaging label Jul 29, 2020
@joshgoebel
Copy link
Member

joshgoebel commented Jul 29, 2020

If it's the main blocker, i could try to help.

To be clear right now the main blocker is that this does not seem necessary and I'm imagining there are other reasonable solutions (which I hope someone will come along and mention). Not to mention that if browserify wants to remain relevant I'm sure it will be adding proper ES6 support - am I mistaken? So it only seems a matter of time until this resolves itself - without requiring any time on our side.

@puzrin Have you filed an issue with browserify on this or does one already exist? A link to it here would be great.

@puzrin
Copy link
Author

puzrin commented Jul 30, 2020

@puzrin Have you filed an issue with browserify on this or does one already exist? A link to it here would be great.

No. I have no concrete suggestions to solve for them.

Ref: https://github.com/babel/babelify#faq

@joshgoebel
Copy link
Member

No. I have no concrete suggestions to solve for them.

Well isn't the problem simply that their build pipeline needs to properly support ES6 code? This seems like an issue that should be filed against their project. What am I missing?

This is one (of many) reasons we moved away from our prior build system... it could not understand ES6 code, so it was a poor choice since we're now using ES6 code in the project.

@puzrin
Copy link
Author

puzrin commented Jul 31, 2020

Well isn't the problem simply that their build pipeline needs to properly support ES6 code? This seems like an issue that should be filed against their project. What am I missing?

They have option to force babel for all sub-deps (see faq link). But that increases build time. I have a lot of deps, but only hljs has such "problem".

This is one (of many) reasons we moved away from our prior build system... it could not understand ES6 code, so it was a poor choice since we're now using ES6 code in the project.

IMO this things are not related anyhow. Right now you can bundle multiple esXX versions, with almost no efforts. May be later, if you decide use advanced es6 feature this may change. But right now drop of es5 looks very strange decision.

@joshgoebel
Copy link
Member

But right now drop of es5 looks very strange decision.

To be clear we didn't "drop" ES5 - so much we upgraded from ES5 to ES6. We've always supporting a SINGLE ECMAscript standard - now we simplify support ES6 rather than ES5.

But that increases build time. I have a lot of deps, but only hljs has such "problem".

Sounds like it shouldn't increase build time much if it's only library that needs to be "down-sampled". And with a good build system there should be ways to cache this as well...

Well isn't the problem simply that their build pipeline needs to properly support ES6 code?

You didn't answer this... our build system (Rollup) works just fine with ES6 code natively... what is the problem with Browserify that it can not bundle ES6 modules?

@joshgoebel
Copy link
Member

@puzrin Please see issue I opened against browserify...

browserify itself does support modern syntax through acorn-node but some transforms may not.

Perhaps it's your transforms, not browersify that's the problem... or you need to install acorn-node?

@goto-bus-stop
Copy link

acorn-node is used internally by browserify so there is no need to install it separately.

From the other issue it sounds like the actual problem is not syntax support in browserify but the fact that it has no simple way to transpile a particular dependency. You can combine https://www.npmjs.com/package/tfilter and babelify to do that. Babel itself filters out node_modules files at some stage I think and needs some configuration to avoid that, but I don't remember what exactly.

@joshgoebel
Copy link
Member

From the other issue it sounds like the actual problem is not syntax support in browserify but the fact that it has no simple way to transpile a particular dependency.

Why would our library need to be transpiled at all if the syntax is not a problem or browserify?

@goto-bus-stop
Copy link

goto-bus-stop commented Jul 31, 2020

Only if you do want to target old browsers, or if you are using outdated transforms.

@puzrin
Copy link
Author

puzrin commented Jul 31, 2020

You didn't answer this... our build system (Rollup) works just fine with ES6 code natively... what is the problem with Browserify that it can not bundle ES6 modules?

You ask me question, that requires dig browserify docs or debug existing app. That's a bit against of my goal. I don't keep in head everything. I have big project, with es6 cjs sources + es5 deps, and with es5 output after bundler pipeline. It's ok for my need, and i would not like to change it until possible.

@joshgoebel
Copy link
Member

@puzrin Looks like this is something you'll need to fix yourself. Seems clear this is specific to your project (or at least fixable if you decided to put in the time) and not truly an issue with browserify OR our use of ES6... of course it's alway great if someone else does the work for you, but unfortunately in this case it won't be us.

Also: You can always build HLJS separately and just use the pre-built JS file outside of your complex build process completely - or just have your build process "concat it" (without any processing). Most build pipelines have an easy way to do that. So it seems there are numerous ways to solve this easily - even if not the exact way you'd like to see it solved.

Good luck.

Closing this issue.

@ljharb
Copy link

ljharb commented Sep 5, 2020

@joshgoebel it is, quite simply, unsafe to transpile code one didn't author, so by your choice to support ES6 and not ES5, you have effectively blocked anyone from using your library in an engine older than your native syntax.

I would strongly urge you to transpile your package to ES5 before publishing if you are interested in those users being able to use your library.

@joshgoebel
Copy link
Member

@ljharb Curious, how is it "unsafe"? I'm not sure I was recommending that in any case. We no longer support any ES5 browsers so moving to ES6 on the client-side made complete sense. I offered several suggestions for getting around this persons legacy build system that they don't want to fiddle with. Simply loading the JS file separately or "concat"ing it "as-is" to a larger build should work just fine for any browsers we still support.

@ljharb
Copy link

ljharb commented Sep 6, 2020

@joshgoebel transpilation is not 1:1, there's all sorts of caveats and edge cases that only the author of the code is qualified to determine if they don't apply.

You're certainly within your rights to drop support for ES5 engines - but please do so knowing that it is effectively impossible for anyone who supports those engines to use your library any further.

@john-doherty
Copy link

Gutted you've dropped support for ES5. I just added your code to a project that has a gulp build system, the es6 code in your lib breaks that. How would i build an ES5 version of this?

@joshgoebel
Copy link
Member

joshgoebel commented Nov 5, 2020

No idea - but I'm 99% sure that's the wrong question. We no longer support ES5 browsers and JS run-times. If Gulp does not support ES6 easily I'd suggest perhaps just dropping the web build of the library directly into your project (in its entirety). IE:

  • Build it first with ./tools/build browser ...
  • Take the distributable and plug it into your build system with some sort of concat plugin (to append it to your build)

Of course you'd likely automate this... (or just make bumping the dependency a more manual task). Of course you could also choose to just serve the highlight.js asset separately.

Even a simple one-line concatenation can work:

cat highlight.min.js your_monolithic_js_bundle.js > final_build.js

@joshgoebel
Copy link
Member

joshgoebel commented Nov 5, 2020

do so knowing that it is effectively impossible for anyone who supports those engines to use your library any further.

This is very untrue and also misleading as there are any number of ways to include our library still (I just mentioned one to @john-doherty)... it's definitely still doable, perhaps just a tiny bit more elbow grease or perhaps none at all (if you're willing to just load the asset separately as MANY websites do - including some very large ones).

@ljharb
Copy link

ljharb commented Nov 5, 2020

That's only the case if highlight.js is a direct dependency (and not a transitive one), and also if your build process allows for that kind of alternative inclusion (most modern ones don't; everything comes in as a normal npm dep, and is run through a bundler).

It's certainly doable - it's computers, we can make them do anything that can be done - but it's effectively impossible because it's not going to be practical to safely do so in the general case.

@joshgoebel
Copy link
Member

and also if your build process allows for that kind of alternative inclusion (most modern ones don't; everything comes in as a normal npm dep, and is run through a bundler).

I'll qualify here: any build process/system that can't easily concat two files together is already a lost cause. :-) I know for sure Gulp can easily do this because I've done it before. If the complaint is really "but i have an extra build step" (Highlight.js) then yes, you would (or you could vendor the library as a static file) - but this should be an easy thing to automate in a reasonable build systems. If not I'd argue the build system is deficient.

That's only the case if highlight.js is a direct dependency (and not a transitive one)

This is a much better a point and arguable a bit harder to solve (I'd imagine). I imagine it would require a small patch to whichever dependency pulled in HLJS to use a the global ES6 version instead. Obviously would be situation dependent.


My opinion here remains the same. This seems to be a niche problem destined to solve itself as ES6 support becomes more mainstream - or people switch away from broken build systems that don't support ES6 (ES2015). It also seems there are ways to do this (with various build systems) just run everything thru Babel, etc, but I don't know about all that. It sounds pretty awful to me to have to do that when your deployment target is a modern browser.

@joshgoebel
Copy link
Member

joshgoebel commented Nov 5, 2020

@ljharb Also (to clarify), often it's not even the build system that's at fault but exactly how people have used/extended/customized it (as so often happens with build systems). For example the original poster here was using Browserify but when I posted an issue upstream asking them if they support ES6... they most certainly totally do, but some specific transforms/plugins may not. So I have a feeling the issue here is often specific legacy build pipelines (as implemented by the user) rather than legacy build systems (overall).

ES6 is no longer a shiny new toy anymore. (it seems major browsers have supported it for 4+ years now and counting)

@puzrin
Copy link
Author

puzrin commented Nov 5, 2020

In browserify i had to enable global option, and that increased build time.

IMO your talks about build systems look like if i start teach you js programming - not very useful and not very relevant.

@joshgoebel
Copy link
Member

joshgoebel commented Nov 5, 2020

IMO your talks about build systems look like if i start teach you js programming - not very useful and not very relevant.

Please, no need for rude or disparaging comments. I don't believe I've been disrespectful here. If "abused" came off poorly I've removed it from my comment (and you have my apologies) - I was using it only in the sense I hear it used regularly "use and/or abuse" in various context - not as any form of personal slander.

I've personally used (and perhaps abused) several of the popular JS build systems (Webpack, Gulp, Rollup) on different projects and I've written the build system we use with Highlight.js entirely from scratch (though it uses rollup). I surely do know something about build systems and JS programming. :-) I've certainly never said I know everything.

So far though I feel I've been mostly correct here in my overall assertion:

  • There are [various] reasonable solutions here other than us providing an ES5 module.
    (Including SUPER trivial solutions for those using us as a first-order dependency.)

In browserify i had to enable global option, and that increased build time.

So a super easy solution, yet one that does indeed have a small cost. So not "effectively impossible".

@joshgoebel
Copy link
Member

joshgoebel commented Nov 6, 2020

Based on years of client experience I never solve the stated request... I focus on underlying problems. In my experience 95% of the time the stated request/desired solution is wrong (or incomplete). Most often the underlying problem has not been correctly understood. 

Correct, right now there's no intention to provide an ES5 build (just to run on ES6 runtimes). I'm unpersuaded anyone needs that build - including you. 🙂 It would not surprise me if the solution for Browserify is a small amount of JS code to customize the build. Yet it's clear I don't yet understand the underlying problem so that makes it harder to help. 

If your target is an ES6 runtime then there's no need to transpile our library and there should also be no need for an ES5 version. It's just the wrong solution. Makes no sense. 

I assume your profit would be your build times returned to 30 seconds. 🙂 But if your done discussing that's fine. 

@joshgoebel
Copy link
Member

joshgoebel commented Nov 6, 2020

@john-doherty

I just added your code to a project that has a gulp build system, the es6 code in your lib breaks that.

It might also help if you clarified your problem a bit more. How exactly does our code break the build? Are you saying that Gulp simply doesn't support ES6 code? Can you show us your Gulp set up?

Edit: I've used Gulp with ES6 code before without issue.

@ljharb
Copy link

ljharb commented Nov 6, 2020

Most build processes do not transpile third-party code, because it's not safe to do that. For example, https://unpkg.com/browse/highlight.js@10.3.2/lib/core.js has arrow functions - it seems like the way you are using them, they're just syntactic sugar, so they'd be safely transpilable, but arrow functions themselves are not safe to naively transpile. They lack a .prototype property, which is both impossible to fake and affects things like using new on them.

Any build process that blindly transpiles third-party code is being reckless and risking breakage to the most vulnerable of users, those using older browsers/engines. It's certainly something that can be opted into, but it is not safe and it's not reasonable to expect anyone to do it. In other words, if you want to drop support for runtimes that, for example, lack arrow functions, please accept that you are forcing your consumers to do the same, because it's simply not practical to audit the entire dep graph and determine whether transpiling is safe, since the vast majority of transpilation is not 1:1.

@puzrin
Copy link
Author

puzrin commented Nov 6, 2020

https://github.com/highlightjs/highlight.js/blob/master/.eslintrc.js#L17

Linter allowed settings are es9 (es2018), not es6.

npx browserslist "defaults, not dead, not ie 11" - that sill require babel settings less than es9.

@joshgoebel
Copy link
Member

@ljharb Did you read my last few messages? I'm not asking anyone to transpile us... I'm suggesting there should be zero reason (or need) to do so. We only support recent versions of Node and green-field web browsers - all ES6 run-times. Most anything in the past few years works just great. What puzrin was seeming to suggest originally is that he only needs an ES5 build to make his build system happy (because it chokes on ES6), but then when he then starts talking about running us thru Babel I get lost.

but arrow functions themselves are not safe to naively transpile. They lack a .prototype property, which is both impossible to fake and affects things like using new on them.

Out of curiosity... how does that make this "unsafe"? Or are you simply saying because the behavior (in edge cases) can't be 100% guaranteed that it's therefore "unsafe"? I can't think of why someone would call new on an arrow function or access prototype for that matter... perhaps a complex system doing introspection of objects could fail in weird ways? I'm sure there are edge cases, I'm just not thinking of them off the top of my head. I agree

If someone were going to transpile us they'd obviously they should take the security concerns into mind, but I just don't see why anyone would...

if you want to drop support for runtimes that, for example, lack arrow functions, please accept that you are forcing your consumers to do the same

We've been VERY clear about that. Do only support ES6 run-time environments, and the library we publish therefore is free to use ES6 features as it sees fit.

@joshgoebel
Copy link
Member

joshgoebel commented Nov 6, 2020

Linter allowed settings are es9 (es2018), not es6.

Indeed. Though this does not require an ES5 build. Though it's potentially a valid reason one could want an ES6-only build. We currently only actually use ES6 code (AFAIK), but just looking now I'm also not sure I have any issues with us making the minimum requirement ES2018. All the desktop targets we support seem to well support ES2018 as well (at the least the features we would use, I'm not sure about FULL support). I'd have to take a closer look at mobile first.

npx browserslist "defaults, not dead, not ie 11" - that sill require babel settings less than es9.

It would be helpful if you mentioned specially which environment you're trying to target - which browser are you actually worried about? Desktop? Mobile? (or do you not really have one in mind?) I think you're supporting a wider range of browsers than we support... Off the top of my head it'd be something more like:

      "targets": {
        "edge": "79",
        "firefox": "60",
        "chrome": "67",
        "safari": "12",
        "node": "12.0"
        }

An ES2018 vs ES6 discussion might be a much more relevant topic. :-)

@ljharb
Copy link

ljharb commented Nov 6, 2020

@joshgoebel yes, but in reality, there is a lot of need to do so. You don't have to agree, of course! However, anyone whose business requires them to support these browsers - which is a great many of us - simply can not use your library without it providing an ES5 build. I agree that anyone targeting an ES6 runtime doesn't need an ES5 build - the problem is that most of us absolutely must target an ES5 runtime, and forgoing the revenue from those users is simply not a viable option.

So, again - not providing an ES5 build screws over your users who must target ES5 runtimes. If that's the choice you're sticking with, so be it, but please do not pretend you're doing anything less than that.

@joshgoebel
Copy link
Member

joshgoebel commented Nov 6, 2020

I don't think we're doing any pretending. We very purposely chose to drop support for IE11 and other non-ES6 browsers. We simply don't have the time (or desire) to support them or their weird quirks. We're supporting 95-98% market share (and growing of course) according to caniuse.com. https://caniuse.com/es6

I can certainly sympathize with business requirements, but someone who needs to support IE11 should probably look for a different highlighting library - or perhaps maintain a small fork (or sponsor one) where they are then responsibility for old browser compatibility and breakage, etc.

So, again - not providing an ES5 build screws over your users who must target ES5 runtimes.

Just curious, is there a specific runtime you're thinking of other than IE11 when you say this?

The fact that this issue (and dropping support for IE11) have received so little interest leads me to believe we're making a reasonable choice for most of our users. Version 10 was released at the beginning of the year and only one or two people have spoken up about IE11 or older browser support.

@puzrin
Copy link
Author

puzrin commented Nov 6, 2020

It would be helpful if you mentioned specially which environment you're trying to target - which browser are you actually worried about?

Browserlist readme advices to worry about npx browserslist "defaults". It costs nothing to wrap hljs call with try/cach to off for ie11, but customizing bunldle process for single package is boring.

An ES2018 vs ES6 discussion might be a much more relevant topic. :-)

As i said earlier, i find your opinion about using bundlers in outer projects too obtrusive, and discussing such things - not optimal time spend to solve introduced problem.

@joshgoebel
Copy link
Member

joshgoebel commented Nov 6, 2020

Browserlist readme advices to worry about npx browserslist "defaults".

I don't know about some of these mobile? options but if you remove IE11 from that list I think there is a lot of ES6 support there. Everything I recognize at least is quite new (all the major browser players, etc) and supports ES6. Opera Mini seems a no and both Baidu & KaiOS are listed as "some compatibility". All of those being < 1% usage.

It costs nothing to wrap hljs call with try/cach to off for ie11, but customizing bunldle process for single package is boring.

Not sure I follow what you're suggesting here.

Linter allowed settings are es9 (es2018), not es6.

Just to confirm the existing codebase is all ES6 [no ES2018 code according to linter] (and likely to stay that way for the near future). So if you were only running us thru Babel because of that mention of ES2018 in the eslint config, you can stop now. :)

@puzrin
Copy link
Author

puzrin commented Nov 6, 2020

For clarity - i don't think you MUST support es5 build. I solved my needs silently, without BS like "you don't understand how opensource works and should not ignore needs of project users". I only don't like mutating context of issue into direction "such demands are silly" (with meaning "silly" demands are written by "silly" users).

Not sure I follow what you're suggesting here.

I just suggest to be more gently with public statements :). What you feel with this is ~ the same as i feel with your posts about "how bundlers should be used" :).

I don't know about some of these mobile? options but if you remove IE11 from that list I think there is a lot of ES6 support there. Everything I recognize at least is quite new (all the major browser players, etc) and supports ES6.

  1. People may not wish to remove IE11 for Babel. They may wish to remove highlighing for IE11.
  2. I don't remember why, but with npx browserslist "defaults, not dead, not ie 11" Babel still produces some pre-ie6 code.
  3. Nobody is happy to change stable bundling process, if es5 output is ok.
  4. Personally, i don't like need to spend time for explaining such things. And without explaining, you declare demands as "not clever". It's normal, if you don't know something. But it's not normal to declare everything else ancient in advance.

Just to confirm the existing codebase is all ES6 [no ES2018 code according to linter] (and likely to stay that way for the near future). So if you were only running us thru Babel because of that mention of ES2018 in the eslint config, you can stop now. :)

See above. I don't like idea to customize bundling process for every specific package. That's not good for my maintenance expectations.

@joshgoebel
Copy link
Member

joshgoebel commented Nov 6, 2020

without BS like "you don't understand how opensource works and should not ignore needs of project users".

Except you just kind of went and said it now... I'd suggest "ignore" is a very strong word. Everyone on the core team volunteers their time to maintain this project. We support 95-98% of browsers commonly in use today. I spend a lot of time on this project meeting a LOT of users needs. I'm sorry if you feel your projects needs have been ignored, but let's not overly generalize.

I only don't like mutating context of issue into direction "such demands are silly" (with meaning "silly" demands are written by "silly" users).

I don't think I used the word "silly" anywhere. I think the language barrier here hasn't helped us any... it's possible you're reading tone or intent into what I'm writing that simply is not there. I tend to be very plain spoken.

What you feel with this is ~ the same as i feel with your posts about "how bundlers should be used" :).

I don't believe any of my remarks about bundlers devolved into personal insults. It probably would have helped if I understood your original goal better (way back when). To me the only non-ES6 browser that even exists (and possibly worth caring about) is IE11... If that makes me dumb, then so be it, but the market share of any others (overall) seems so close to 0 as to almost be a rounding error. So your talking about ES5 apart from IE11 really just went over my head (my apologies). For a long time I thought you wanted to build ES5 just because your build system mandated it for some strange reason - which made very, very little sense. :-)

Secretly I wondered if you really still just wanted IE11 support (else why compile to ES5? lol)... but now in re-reading it's clear what you were really saying was "Can I still use this in all my other ES5 browsers, just not IE 11?" To which the answer is still no, the market share is just too small to make them worth supporting. Without using or knowing anything about those niche browsers it's simply impossible to make sure users would have a great experience. So along with IE11, they are also now unsupported.

People may not wish to remove IE11 for Babel. They may wish to remove highlighing for IE11.

I wasn't being literal. I was only pointing out that I think most the browsers on that list (with any real general usage) already support ES6, with the exception of IE11. If you were able to drop your IE11 and Opera Mini requirements you might be able to get away with an ES6 build. (not sure, but maybe)

Nobody is happy to change stable bundling process, if es5 output is ok.

Sure, if you're happy with 50s cold start instead of 30s cold start then I suppose it's "ok". Like most things, it's about tradeoffs. In this case the simple "one line fix" doubles the cold start time... vs a slightly more complex bundle that would keep the fast cold start.

And without explaining, you declare demands as "not clever". It's normal, if you don't know something. But it's not normal to declare everything else ancient in advance.

I think I've explained a lot... perhaps I missed a question? I also don't see where I've used the word "clever" anywhere. And not sure what I've declared ancient... ES5? Generally, for 95-98% of browser share it is... though to be clearer relevancy might be a better metric than age.

@joshgoebel
Copy link
Member

joshgoebel commented Nov 6, 2020

@puzrin You might also be interested in #2756. It's only a matter of time until we're using ES6 features you can't transpile to ES5 at all... and then you're looking at maintaining your own fork of the library... and that's also why we don't officially support ES5 anymore - it's about to potentially get much harder to do so. Lack of time, lack of team interest, lack of real demand, tiny (and shrinking) market share, and we know that we're likely to break it in the near future anyway.

You're welcome to transpile it as long as you can (if that works) - we certainly can't stop you. But we didn't randomly drop support for ES5. It was considered and it was a signal of where things are headed... so while transpiling is working for you right now I'd be spending some time planning your next move...

If IE11 (and other ES5) support is mission critical for you I'd seriously consider looking at https://prismjs.com/ which so far seems interested in IE11 support. Nope, never mind, they are looking to drop IE11 (and likely ES5) support soon as well. PrismJS/prism#1578

@puzrin
Copy link
Author

puzrin commented Nov 6, 2020

Except you just kind of went and said it now... I'd suggest "ignore" is a very strong word. Everyone on the core team volunteers their time to maintain this project. We support 95-98% of browsers commonly in use today. I spend a lot of time on this project meeting a LOT of users needs. I'm sorry if you feel your projects needs have been ignored, but let's not overly generalize.

Again, i'm ok if you ignore demands of es5. I'm no ok HOW you do that. Probably, due language barrier i can't express my thoughts good enough.

I don't think I used the word "silly" anywhere. I think the language barrier here hasn't helped us any... it's possible you're reading tone or intent into what I'm writing that simply is not there. I tend to be very plain spoken.

Since i have low telepathy skills, i react not to what you think, but how i understand what i read :). From my point of view, package demands could be isolated. Propagating those to app requirements (discussing bundlers) is out of scope (IMO).

People may not wish to remove IE11 for Babel. They may wish to remove highlighing for IE11.

I wasn't being literal. I was only pointing out that I think most the browsers on that list (with any real general usage) already support ES6, with the exception of IE11. If you were able to drop your IE11 and Opera Mini requirements you might be able to get away with an ES6 build. (not sure, but maybe)

IMO operating with guesses is not constructive. There is wide-used browserslist "default" preset. You may not like it or disagree with it. But some people prefer rely on it without care about details of each browser.

Nobody is happy to change stable bundling process, if es5 output is ok.

Sure, if you're happy with 50s cold start instead of 30s cold start then I suppose it's "ok". Like most things, it's about tradeoffs. In this case the simple "one line fix" doubles the cold start time... vs a slightly more complex bundle that would keep the fast cold start.

Those comment was given in context you were giving advices without knowing details and side effets. Any your decisions about hljs are ok. But attempts to "intrude" into outer processes can make someone feel uncomfortable.

No, i'm no "happy" with increased cold start time. I'm just "ok" with it. And if i decide to spend time for improve, i will try multithreading support instead of written recommendations.

And without explaining, you declare demands as "not clever". It's normal, if you don't know something. But it's not normal to declare everything else ancient in advance.

I think I've explained a lot... perhaps I missed a question? I also don't see where I've used the word "clever" anywhere. And not sure what I've declared ancient... ES5? Generally, for 95-98% of browser share it is...

Missed question is, that i had to spend notable time for bundler changes and talks without reasonable needs, from my point of view :). You forced users like me to participated in hljs process, instead of allow make a choice, what kind of bundle to use. That's too pushing. At this moment, bundling to es5 is completely automated process with almost zero cost for you.

@puzrin
Copy link
Author

puzrin commented Nov 6, 2020

@puzrin You might also be interested in #2756. It's only a matter of time until we're using ES6 features you can't transpile to ES5 at all... and then you're looking at maintaining your own fork of the library... and that's also why we don't officially support ES5 anymore - it's about to potentially get much harder to do so. Lack of time, lack of team interest, lack of real demand, tiny (and shrinking) market share, and we know that we're likely to break it in the near future anyway.

When you use non-transpileable es6 features, that will be another story. But right now you compare "not existing potential features" with really existing "difficulties". IMO such kind of arguments is "black rhetoric" (when goal of discussion replaced with goal to prove "i'm right and you are not").

@joshgoebel
Copy link
Member

joshgoebel commented Nov 6, 2020

when goal of discussion replaced with goal to prove "i'm right and you are not"

Obviously you're understanding me wrong. I was simply giving you a clear heads up. You seemed to feel taken by surprise when ES5 was removed (in a major release). So despite you're setup already being unsupported I'm giving you warning now so when things break you can't say it came as any surprise. You could be doing something about that today vs waiting for it to break tomorrow. But obviously that's your decision. :-)

Justifying why we dropped support for ES5 isn't the same as claiming "I'm right". It's possible a different maintainer (in another universe) would have made an entirely different decision. It was the right decision for our team I think, but I don't think it's right or wrong in any universal sense of the word. It simply is what it is.

@joshgoebel
Copy link
Member

From my point of view, package demands could be isolated. Propagating those to app requirements (discussing bundlers) is out of scope (IMO).

Now that I clearly understand your goal I probably would not have said much of what I said and simply stuck with "Sorry, ES5 is simply not supported for IE and other browsers." :-) Oh well, water under the bridge.

IMO operating with guesses is not constructive. There is wide-used browserslist "default" preset. You may not like it or disagree with it. But some people prefer rely on it without care about details of each browser.

Every suggestion is a guess until properly researched, no? :-) I was merely pointing out that browserlists own "default" preset itself seems to mostly agree with our policy (once you remove a few outliers) to drop ES5. When it will finally match completely I obviously have no idea. I'll avoid guessing. :-)

Those comment was given in context you were giving advices without knowing details and side effets. Any your decisions about hljs are ok. But attempts to "intrude" into outer processes can make someone feel uncomfortable.

I offered some suggestions - most people appreciate this. You may take them or leave them. There is no reason to feel uncomfortable.

i will try multithreading support instead of written recommendations.

That's an idea! :-)

...i had to spend notable time for bundler changes... ... forced users like me to participated in hljs process...

No one is forcing you to change your bundle setup or forcing you to participate. You're free to leave.

At this moment, bundling to es5 is completely automated process with almost zero cost for you.

So it's ok when you make the suggestions, but not when I do? :-) Seems a bit unfair. :-) I've already addressed this zero cost myth. Implementation, maintenance and support costs are real costs. Time it takes to make hotfixes to deal with stupid ES5 only bugs in older browsers is real time that we don't get back. Asking people "are you using the ES5 or ES6 build?" makes diagnosing and triaging problems harder. "almost zero in a perfect world", maybe. "almost zero" in the real world in 2020, definitely not.

@joshgoebel
Copy link
Member

This is past the point of being constructive though. You will continue to make the decisions you feel are best for your project, and we will also do the same. If there are any final thoughts lets air them then return this thread to resting in peace or perhaps helping @john-doherty if he still needs assistance :-)

@puzrin
Copy link
Author

puzrin commented Nov 6, 2020

I'm giving you warning now so when things break you can't say it came as any surprise. You could be doing something about that today vs waiting for it to break tomorrow. But obviously that's your decision. :-)

I have a bit different long-term plan. Sate of browsers is not static. When es6 become really mandatory in hljs, browserslist may give complete different result, and changes may be applied wihout pain. Strategy of "doing nothing until possible" looks more attractive for me.

So it's ok when you make the suggestions, but not when I do? :-) Seems a bit unfair. :-)

Weight of suggesion is direcly related to "who will pay for it".

  • When i suggest es5 build - i'm ready to help with PR and not bother you about es5-specific problems. It seems i'm responcible for my proposal.
  • When i get advice to rewrite bundler... person who give such advice is not responsible for it.

Logic is simple and honest, IMO :)

I've already addressed this zero cost myth. Implementation, maintenance and support costs are real costs. Time it takes to make hotfixes to deal with stupid ES5 only bugs in older browsers is real time that we don't get back.

My suggestions are based on my experience. I respect your time and would never suggest "difficult" unpleasant things. See how many project i support in my orgs. IMO i have reasonable stat about transpile to es5. If question would be only in doing PR - i could do that.

I suggest a minimalisic thing - completely automated es5 build without support, "use on your own risk and not with ie11". That could be enough until better time come.

And of cause, i'm ok with simple answer "i don't like to support es5 build" :). In my eyes personal preferences are normal and don't need technical proofs.

@john-doherty
Copy link

Hi @joshgoebel, you're right that gulp was not the issue, it's more the gulp js minifier I'm using. The es6 only option is a blocker in my case, unless I was willing to do more than copy and paste it into my project. I did not have time, so went with prismjs.

I think that's the concern here, it's a potential barrier for new users - which could hurt future adoption. I don't know the codebase, but cant you just add a minified es5 version?

@joshgoebel
Copy link
Member

Strategy of "doing nothing until possible" looks more attractive for me.

👍🏼 There are definitely some real merits to that approach for sure. :-) The UTF-8 stuff might land in Dec/Jan/Feb timeframe. We'll see how it goes.

Weight of suggestion is direcly related to "who will pay for it".

That is a good point. It would carry more weight if you were on the core team though and also required to deal with support issues, fixing ES5 bugs, supporting these obscure browsers, issuing hot-fixes, etc... I find that all too often the PR is only the "tip of the iceberg" when it comes to the actual effort of getting a thing done. Perhaps your experience is different.

I think personally I'm much less comfortable with "use on your own risk" vs "it works on modern browsers, guaranteed". Obviously a matter of personal taste and perhaps OSS philosophy in general. We're not preventing anyone from transpiling (or even using IE11). We're just saying "it might turn out poorly, so we're not going to assist".

Even if I were more receptive we already dropped ES5 support in April. We have a plan to use ES6 features soon that (afaik) either make ES5 not transpile not possible or require even more developer effort to maintain. So adding it back with the caveat "use at your own risk" just to take it away in a few months makes very little sense.

i'm ok with simple answer "i don't like to support es5 build"

Well, but I also think our reasons are sound, even if you happen to disagree or think it's the wrong choice. It's a decision made with a reason, not just a whim. :-)

@joshgoebel
Copy link
Member

joshgoebel commented Nov 6, 2020

I don't know the codebase, but cant you just add a minified es5 version?

No. See entire discussion above if you want more context. Supporting legacy build system plugins (when there are good alternatives available) isn't a priority. FYI: There are Prism issues talking about dropping support for IE11 and ES5 also. (though I dunno their thoughts on transpiling)

it's more the gulp js minifier I'm using.

FYI: Terser works great with Gulp and fully supports ES6. Just FYI. We use terser for our own builds of Highlight.js. So if you decided to switch in the future you could do so easily with Gulp + Terser as your minifier.

I did not have time, so went with prismjs.

Prism.js is a great project with much to recommend it. :-)

@ljharb
Copy link

ljharb commented Nov 6, 2020

ftr, that prism issue you linked hasn't had much discussion in the last 2 years, except to suggest using a build process to preserve support for those environments.

@joshgoebel
Copy link
Member

joshgoebel commented Nov 6, 2020

I edited my last post to remove the "soon" and make it clear I'm not speaking for the Prism team.

I didn't pay much attention to the dates more so than the overall direction... that ES5 is on the way out. I'm also not sure the age of the issues in this case is a positive sign. :-) As I said they might be more willing to transpile - though again I'm not 100% sure why if they decide to drop IE11 support. There were a bunch of interconnected issues about the future of Prism - I'm not sure if you looked at them all. Some would really like to user regex look-behinds (as would we)... that of course will certainly break ES5, though that's likely still a year or more out due to Safari's lagging support.

@john-doherty
Copy link

I saw it had regular commits and was still use by Stripe so grabbed that... If this was my project, my concern would be that dropping ES5 would eliminate this tool from been used by any SaaS/API company that wants to sell to large orgs (many of which are still running old browsers/devices).

@joshgoebel
Copy link
Member

joshgoebel commented Nov 6, 2020

@john-doherty It's possible there might be room for a docfix here somewhere... ie "Using Hightlight.js with Gulp"... where we could mention to use Terser to minimize (vs xyz, which doesn't support ES6)... the problem is often build systems and their setups can quickly get very complex... but perhaps there might still be value in a list of "hints" to guide people... I just don't know where that would go off the top of my head... perhaps a build system guide? "Hints for using Highlight.js with different build systems"?

dropping ES5 would eliminate this tool from been used by any SaaS/API company that wants to sell to large orgs (many of which are still running old browsers/devices).

I think the ecosystem has room for more than one alternative - with different alternatives having different goals and focus points. We're better at some things, Prism is better at others - like being tiny, and supporting IE11. Users can choose which things they value more. We don't have to solve the highlighting problem for every last person, organization, and browser on the planet. We're ok being Apple and removing the floppy drive first. :-) We're happy solving (and well supporting) the 95% case.

Perhaps for such a company Prism.js would be a better fit. Or perhaps if they are truly a profitable SaaS business with a dev team and a support staff then they could maintain (and support, and open source, etc) their own ES5 port of the library. It's not (currently) an insurmountable task (by any means) - it's just going to get harder and harder as time moves forward.

@joshgoebel
Copy link
Member

https://babeljs.io/docs/en/babel-plugin-proposal-unicode-property-regex

Wow, truly, people are ingenious... though the output of something like /\p{ID_Start}/u is just SCARY and I wonder if it wouldn't have potential performance implications (though maybe that's what happens internally anyways and \p is just sugar). And of course sorting out subtle bugs if the behavior between ES5 and ES6 ever diverged could potentially be quite an adventure. :-)

@puzrin
Copy link
Author

puzrin commented Nov 6, 2020

The UTF-8 stuff might land in Dec/Jan/Feb timeframe. We'll see how it goes.

If you mean /u regexp flag - this can be transpiled partially or just left "as is" with "throw exception in old browsers". Minimal outer try/catch wrapper should silently dim regression. I mean, some old bowser will not highlight code, but your package will not trash everything.

Usually, there are possibilities to keep acceptable compatibility (not full, but still useable) with es5 transpiler without pain for development process. If you need help with this - feel free to kick me anytime with questions or PR requests.

@puzrin
Copy link
Author

puzrin commented Nov 6, 2020

Wow, truly, people are ingenious... though the output of something like /\p{ID_Start}/u is just SCARY and I wonder if it wouldn't have potential performance implications (though maybe that's what happens internally anyways and \p is just sugar). And of course sorting out subtle bugs if the behavior between ES5 and ES6 ever diverged could potentially be quite an adventure. :-)

There are no serious perf issues. I'm author of this madness, and it works well. Not sure if 100% can be transpiled (problem is negative classes and /y).

But, for this package i would prefer throw exception instead of bundle size increase. Then the only case to fight is syntax errors on script load. Workaround is to use new RegExp('foobar', 'u') instead of /foobar/u. Probably this can be automated with small babel plugin.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package/build Issues relating to npm or packaging
Projects
None yet
Development

No branches or pull requests

5 participants