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

[CLEANUP] Remove IE11 support #19558

Merged
merged 4 commits into from
May 27, 2021

Conversation

simonihmig
Copy link
Contributor

@simonihmig simonihmig commented May 25, 2021

Remove IE11 deprecation warnings and tests, as per RFC 685.

Removing the transpilation for IE11 unveiled some mismatch of other transpilation settings vs. the testing environments:

  • Safari tests were failing, as babel settings were "last 2 Safari versions" (which according to latest caniuse data is 14.1 + 14), but BrowserStack was configured for Safari 12.1. Changed this to 14.
  • node tests were as well failing with some syntax errors. Added a node: 'current' setting to targets, but only when running node tests, to keep browser tests untouched.

@simonihmig
Copy link
Contributor Author

Hm, the CI UI seems to be somehow broken here, indicating that there is a pending "Browserstack Tests (Safari, Edge, IE11)" job, which actually does not exist (has been renamed, and the renamed one exists and is green). When you click on the "Checks" tab, all jobs are listed as expected and green. 🤔

@simonihmig
Copy link
Contributor Author

Btw, there are probably one or two more iterations possible here, to remove remaining IE11 cruft, like workarounds for missing APIs. But I wanted to keep the scope here to the most essential changes.

@@ -3,7 +3,6 @@ const allSupportedBrowsers = [
'last 2 Firefox versions',
'last 2 Safari versions',
'last 2 Edge versions',
'ie 11',
];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure if we actually need to keep these two separate lists, as they are mostly the same now (with Edge basically being Chrome)?

@SergeAstapov
Copy link
Contributor

SergeAstapov commented May 26, 2021

@simonihmig this is definitely 👍!

However, per PRF #685 (and Ember.js Browser Support Policy ultimately) Safari 12 is supported by Ember.js.

I wonder if CI still should be configured for Safari 12?

@simonihmig
Copy link
Contributor Author

However, per PRF #685 (and Ember.js Browser Support Policy ultimately) Safari 12 is supported by Ember.js.

Oh, right, good catch! Thanks!

I did not intend to implement that RFC fully here, more to focus on getting rid of IE for unblocking further refactorings and other semi-related PRs (#19552). But at least we shouldn't cause regressions in test coverage here.

So I reverted the BrowserStack config change, and added Safari 12 to targets, which now also makes the Safari tests pass again!

Uh, blueprint tests are failing somehow, but that seems unrelated

@simonihmig simonihmig mentioned this pull request May 26, 2021
19 tasks
@rwjblue
Copy link
Member

rwjblue commented May 27, 2021

Due to some complications with 3.27's migration to use real modules (and the resulting massive number of deprecations being triggered) we are not 100% certain (yet) that 4.0 will be the version just after v3.28.

In order to move ahead with v4.0 cleanup efforts (like this one), I've pushed a new branch that can serve as the target for breaking change PR's: https://github.com/emberjs/ember.js/tree/v4-cleanup.

I've updated this PR to target that branch, can you rebase against that branch and push an update?

Expand for an example of the commands needed for that rebase.
# starting from this PR's branch

git fetch origin
git rebase origin/v4-cleanup

# fix any conflicts

# push the rebase
git push -f

Those steps should be roughly what you need, but might need some tweaks based on your local repository setup (e.g. if you don't use origin for the main emberjs/ember.js repo you might have to use another remote name there).

Thank you for helping us push things forward!

@rwjblue rwjblue changed the base branch from master to v4-cleanup May 27, 2021 15:02
@simonihmig
Copy link
Contributor Author

Done

@rwjblue rwjblue changed the title [Ember 4.0] Remove IE11 support [CLEANUP] Remove IE11 support May 27, 2021
@rwjblue rwjblue merged commit 89a58e6 into emberjs:v4-cleanup May 27, 2021
@simonihmig simonihmig deleted the remove-ie11-support branch May 28, 2021 23:14
@nlfurniss nlfurniss mentioned this pull request Jul 13, 2021
58 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants