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

revert remove contextual overflow #4102

Conversation

tanhauhau
Copy link
Member

@tanhauhau tanhauhau commented Dec 12, 2019

revert #4095

Fixes #4098, Fixes #4120, Fixes #4114

  • revert while keeping newly added test in 4095
  • added another test for bitmask overflow slot

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Looks great! What a simple, elegant fix!

@ghost
Copy link

ghost commented Dec 12, 2019

@Conduitry,

This PR should be preferred over #4101 to address #4098 as I believe it fixes all instances of updates within actions.

@ghost
Copy link

ghost commented Dec 17, 2019

Also fixes #4120... can we get this merged soon?

@ghost
Copy link

ghost commented Dec 17, 2019

@tanhauhau,

Here's some runtime tests to trap additional cases found by community members.
I didn't know if I could push onto your branch for the PR, so I branched off of yours and added it.

https://github.com/dkondrad/svelte/commit/0b71c4baa9ffb46ef5980b847c445c193a8c3e9c
https://github.com/dkondrad/svelte/commit/d13d725fd04ecdeea396e7ce207399e161d92a3a

@tanhauhau
Copy link
Member Author

omg thanks @dkondrad for the new test cases

@tanhauhau
Copy link
Member Author

ping @Conduitry @Rich-Harris can help review this PR?

@Conduitry Conduitry merged commit a8b306f into sveltejs:master Dec 18, 2019
@tanhauhau tanhauhau deleted the tanhauhau/revert-remove-contextual-overflow branch December 18, 2019 01:17
taylorzane pushed a commit to taylorzane/svelte that referenced this pull request Dec 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants