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

Is ckeditor.compat.js really needed? #2149

Closed
wwalc opened this issue Aug 28, 2017 · 7 comments
Closed

Is ckeditor.compat.js really needed? #2149

wwalc opened this issue Aug 28, 2017 · 7 comments
Labels
package:build-classic type:improvement This issue reports a possible enhancement of an existing feature.
Milestone

Comments

@wwalc
Copy link
Member

wwalc commented Aug 28, 2017

Initially I wanted to report it as a docs issue in https://github.com/ckeditor/ckeditor5 but then thought that perhaps we are offering something that has no sense at this moment? I mean, does the transpiled distribution ckeditor.compat.js bring any additional browser support? Isn't it that we are currently using features that only browsers with ES6 support include?

I'm having a hard time trying to figure out which browser compatibility applies to which distribution:

  • build/ckeditor.js
  • build/ckeditor.compat.js

Looking at included files and browser compatibility I couldn't find this info.

What I'm worried about is that people will load by default ckeditor.compat.js just because it offers "additional browser" support, causing in the end that CKEditor 5 will work slower in environments where it could run faster otherwise.

@fredck
Copy link
Contributor

fredck commented Aug 28, 2017

The idea is keeping ckeditor.js as a pure ES6 script, while ckeditor.compat.js is the transpiled version of that same script with the minimum necessary changes to work in our whole range of compatible browsers.

The problem is that the "minimum necessary" may be a lot and the file may be much bigger.

It is a great idea to give the option to developers to decide which file to load depending on the user's browser. In the other hand, the documentation must be very clear about this and it is not, in fact. So, it sounds that this is mainly a documentation issue.

@Reinmar
Copy link
Member

Reinmar commented Aug 31, 2017

@wwalc is right – I figured this out after we introduced the compat build (and perhaps reported it somewhere). All browsers that we do support have support for ES6. So I can't point you to a browser which might need the compat build. This will change only if we'll start supporting some legacy browsers like IE11 or something I'm not even aware of right now.

I'm all for removing the compat build.

@fredck
Copy link
Contributor

fredck commented Aug 31, 2017

Well, if there is no transpilation happening at this stage, it doesn't make sense having "compat", indeed.

The good thing is that, if we'll ever introduce it in the future, it should be fine, as developers will have an "opt-in" way to decide whether they will want compatibility with the new legacy browsers supported.

@Reinmar
Copy link
Member

Reinmar commented Aug 31, 2017

Well, if there is no transpilation happening at this stage, it doesn't make sense having "compat", indeed.

To make it clear – that build uses transpilation. It's that no browser needs that.

And to make it even more clear – the kind of compatibility issues that we encountered are with HTML APIs, not the language itself. E.g. missing Symbol.iterators, missing Range() constructor, or broken clipboard API, etc (mostly in Edge). Transpilation doesn't help here unfortunately.

@fredck
Copy link
Contributor

fredck commented Aug 31, 2017

Does this mean that we should move support for Edge to "compat" and leave ckeditor.js without any transpilation?

@Reinmar
Copy link
Member

Reinmar commented Aug 31, 2017

What do you mean by "move"?

Edge works fine with ckeditor.js already and ckeditor.js doesn't use any transpilation.

@fredck
Copy link
Contributor

fredck commented Aug 31, 2017

Ah, sorry... got confused when reading your previous comment, misreading the last phrase in it.

So it's all set then to remove compat for now, I assume.

@mlewand mlewand transferred this issue from ckeditor/ckeditor5-build-classic Oct 8, 2019
@mlewand mlewand added status:confirmed type:improvement This issue reports a possible enhancement of an existing feature. package:build-classic labels Oct 8, 2019
@mlewand mlewand added this to the iteration 11 milestone Oct 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:build-classic type:improvement This issue reports a possible enhancement of an existing feature.
Projects
None yet
Development

No branches or pull requests

4 participants