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

Update ES6 results for IE11 #1312

Merged
merged 5 commits into from May 6, 2020
Merged

Conversation

afmenez
Copy link
Contributor

@afmenez afmenez commented Jun 11, 2018

After the latest fixes from @Perelandric , it is now possible to run all tests with IE11 (the table layout is still broken, but at least the results are shown). This patch fixes all results that were wrong.

@zloirock
Copy link
Collaborator

zloirock commented Jun 11, 2018

Uint8ClampedArray has been introduced in the build KB2929437, it's missing in early IE11 versions. So, at least, first 2 results should not be added.

@@ -5323,6 +5323,11 @@ exports.tests = [
typescript1: typescript.corejs,
firefox2: false,
firefox4: true,
ie11: {
val: true,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather false. Also, .subarray test depends on Uint8ClampedArray.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this value supposed to reflect initial release or the version that people are probably using? I believe it is the later.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean the note should be added to .subarray too? Wouldn't that be confusing? Maybe the test should be changed to avoid the dependency.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For IE, it’s the latest in that major line.

Copy link
Collaborator

@zloirock zloirock Jun 11, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this value supposed to reflect initial release or the version that people are probably using? I believe it is the later.

Just an example: we will set is as true. babel-preset-env will load this result. For IE11 on Uint8ClampedArray preset-env with useBuiltIns option will not add a polyfill. But a serious part of users has un-updated versions of IE11.

Do you mean the note should be added to .subarray too?

I don't care, but with current tests it should be false.

Maybe the test should be changed to avoid the dependency.

IIRC it's already was proposed in another issues.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current column represents "IE 11 latest". If you want negative results for non-latest IE 11, then you're welcome to add a column.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Me? -) Interesting position. The current column is just IE11 with release date 2013-10-17. It's definitely not "IE 11 latest".

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

preset-env could override results. Also ideally it should use something what fits better for it than compat-table, which purpose never was to provide data for preset-env or anything else.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

preset-env could override results.

It's additional special cases. Why?

Also ideally it should use something what fits better for it than compat-table

Definitely. But currently we haven't any good alternative.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a) the release date is for the first version, results always reflect the latest
b) supporting preset-env is ancillary and NOT something that this repo needs to cleave to, especially not when it changes the meaning this project's test columns have had historically.

Copy link
Collaborator

@zloirock zloirock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix false positive for early IE11 versions.

Copy link
Collaborator

@chicoxyzzy chicoxyzzy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that notes cover all necessary info so I'll approve

@chicoxyzzy
Copy link
Collaborator

@zloirock are you ok with latest changes?

@zloirock
Copy link
Collaborator

@chicoxyzzy nope, see my argumentation above - nothing was changed.

@chicoxyzzy
Copy link
Collaborator

chicoxyzzy commented Aug 29, 2018

Is everyone ok if we'll show these features as unsupported and will add a note that in latest versions of IE these features was added?

@ljharb
Copy link
Member

ljharb commented Aug 29, 2018

No, i want them to show as supported if they’re available in the latest version, per precedent.

@mgol
Copy link
Contributor

mgol commented Jul 25, 2019

This PR has been opened a year ago. Can we get a decision here so that it can be landed in whatever version gets accepted?

Copy link
Collaborator

@chicoxyzzy chicoxyzzy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@zloirock
Copy link
Collaborator

I wrote my objections above. I could accept only false with a note or adding a new version. I don't wanna break the web.

@ljharb
Copy link
Member

ljharb commented Jul 25, 2019

It won't break the web; the columns mean "latest version". Anyone not using the latest version of the thing in the column is already broken if they're relying on this data.

@zloirock
Copy link
Collaborator

compat-table is a data source not only about the latest versions of things. As I wrote above, the current column is just IE11 with release date 2013-10-17. And many people relying on this data as on data about all IE11 versions.

@ljharb
Copy link
Member

ljharb commented Jul 25, 2019

Then it sounds like we should add a new column for the latest IE 11, and mark the current one as very obsolete.

@zloirock
Copy link
Collaborator

zloirock commented Jul 25, 2019

I agree. But just "obsolete" could be better.

@afmenez
Copy link
Contributor Author

afmenez commented Jul 25, 2019

I don't believe it is productive to add columns for minor versions. How about changing the results to "flagged", keeping the notes on the current PR?

@chicoxyzzy
Copy link
Collaborator

chicoxyzzy commented Jul 25, 2019

compat-table is a data source not only about the latest versions of things. As I wrote above, the current column is just IE11 with release date 2013-10-17. And many people relying on this data as on data about all IE11 versions.

So we should create issue / PR against babel-present-env to override this as it overrides other things. If any tool uses compat-table data as is, than it's probably already broken

@chicoxyzzy
Copy link
Collaborator

chicoxyzzy commented Jul 25, 2019

Everywhere else we indicate latest status. If not, we'd need to mark a bunch of Safari, Chrome and Firefox features as false because of temporary bugs in minor versions and patches.

@ljharb
Copy link
Member

ljharb commented Jul 25, 2019

^ ftr, that matches my intuition as well as babel-preset-env reality, and is why I think @zloirock's position here is objectively wrong.

@zloirock
Copy link
Collaborator

@chicoxyzzy yes, compat-table contains incomplete data, but it would be better just improve this data instead of making it even more broken and required even more special cases. It's not a problem for me since for core-js@3 I made core-js-compat, but it will cause problems in many other cases.

The case from this PR is adding new features, not fixing bugs, so it's a semver minor, not patch. So incorrectly compare it with other browsers, the issue here in a versioning / naming MS browsers.

@ljharb
Copy link
Member

ljharb commented Jul 26, 2019

The best thing to do is take broken use cases - like “not the latest version of IE 11” and make them more broken, precisely so the users wrongly relying on this nonexistent property have more incentive to stop doing so.

It’s unreasonable to insist on maintaining a property that has never existed and does not currently exist.

@chicoxyzzy
Copy link
Collaborator

I still think that we should merge this.

@existentialism @nicolo-ribaudo @JLHwung how hard it would be to handle this change in babel-preset-env if we'll merge this?

@nicolo-ribaudo
Copy link
Contributor

It wouldn't be too hard, we can hard code the old data.

@ljharb
Copy link
Member

ljharb commented May 5, 2020

Typically, overriding a block isn't something I'd ever support; however there's extenuating circumstances here, and we don't want to have to wait a year or more, so if @chicoxyzzy agrees, I'm ok with that.

data-es6.js Outdated Show resolved Hide resolved
@chicoxyzzy chicoxyzzy merged commit 142bb3f into compat-table:gh-pages May 6, 2020
@afmenez afmenez deleted the es6ie11upd branch May 6, 2020 22:07
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

Successfully merging this pull request may close these issues.

None yet

6 participants