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 jQuery integration in EventDispatcher #19552

Closed

Conversation

simonihmig
Copy link
Contributor

Second part after #19551 to remove deprecations of RFC386.

@simonihmig
Copy link
Contributor Author

Ok, so there are 4 failings tests for IE11. These are all related to the also deprecated moveEnter/Leave events. They fail due to the inability to override Event.target in IE11, which has been discussed in #17228. As was also confirmed in that issue, tests in CI were only passing for IE11 because they only ran in jQuery integration mode. Now with that removed, they start to fail in CI.

Again, we cannot fix this behavior in IE11, and as these events are deprecated anyway, it seems unreasonable to spend more time on fixing/disabling those tests here. The removal of support for these events if already underway in #19529 (/cc @btecu), so I would suggest we try to land this first, and then rebase this PR which should then make CI happy.

@pzuraq
Copy link
Contributor

pzuraq commented May 24, 2021

We probably should have a separate PR removing IE11 from CI. Once we start landing breaking changes on canary, we won't need to support IE11 anymore (as it's deprecated in 4.0), so no need to test against it.

@simonihmig
Copy link
Contributor Author

Once we start landing breaking changes on canary

I wondered about this, so when actually is this going to happen? The package.json in master is saying this is already 4.0.0-alpha. But seems this is not set in stone? Is there still a Core team decision pending?

@pzuraq
Copy link
Contributor

pzuraq commented May 24, 2021

@simonihmig yeah I'm trying to figure this out now. I think what could possibly happen is, we start landing the cleanup/fixes for 4.0 on canary and move forward. If we do end up needing to have another 1-2 releases after 3.28, we could re-release the 3.28 LTS and continue supporting it on a different branch. We've done this before when a beta was particularly unstable and we had to push it back, so I think we could do something similar here if we needed to, but I want to get consensus from the rest of the team about that.

@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:01
@simonihmig simonihmig force-pushed the remove-jquery-eventdispatcher branch from dce4061 to 6753f42 Compare May 27, 2021 15:51
@simonihmig
Copy link
Contributor Author

Done

@rwjblue
Copy link
Member

rwjblue commented May 27, 2021

OK, I think we need another rebase to drop IE11 from the browser stack setup and then this should be good, right?

@simonihmig simonihmig force-pushed the remove-jquery-eventdispatcher branch from 6753f42 to c452cdc Compare May 27, 2021 16:21
@simonihmig simonihmig changed the title [Ember 4.0] Remove jQuery integration in EventDispatcher [CLEANUP] Remove jQuery integration in EventDispatcher May 27, 2021
@simonihmig
Copy link
Contributor Author

@rwjblue yep, did that, green now!

@rwjblue rwjblue closed this Jul 18, 2021
@rwjblue rwjblue deleted the branch emberjs:v4-cleanup July 18, 2021 02:05
@mixonic
Copy link
Member

mixonic commented Jul 18, 2021

@simonihmig this PR was auto-closed when v4-cleanup was merged to master. Can you re-open for us to review/merge against master? This is excellent work and I don't want to lose it.

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.

4 participants