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.

Sign up for GitHub

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

Jump to bottom

Reverse hook order for all but onbeforeupdate #2297

Merged
merged 2 commits into from
Nov 27, 2018
Merged

Reverse hook order for all but onbeforeupdate #2297

merged 2 commits into from
Nov 27, 2018

Conversation

dead-claudia
Copy link
Member

Description

Call attrs hooks before component hooks

  • Drive-by: onbeforeupdate prevents subtree redraw if either hook returns false, not both.

Motivation and Context

Fixes #2270.

How Has This Been Tested?

Updated tests appropriately

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation change

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • I have updated docs/change-log.md

@dead-claudia dead-claudia requested a review from pygy as a code owner November 14, 2018 06:12
@dead-claudia dead-claudia added Type: Bug For bugs and any other unexpected breakage Type: Breaking Change For any feature request or suggestion that could reasonably break existing code labels Nov 14, 2018
@dead-claudia dead-claudia added this to the 2.0.0 milestone Nov 14, 2018
@dead-claudia
Reverse hook order for all but onbeforeupdate
e50fe7b 
- Drive-by: `onbeforeupdate` prevents subtree redraw if *either* hook
  returns `false`, not *both*.
@dead-claudia
Copy link
Member Author

dead-claudia commented Nov 21, 2018 •
edited
Loading

Ping @barneycarroll: I know you were involved in the discussion, so could you take a look at this?

barneycarroll
barneycarroll approved these changes Nov 27, 2018
View reviewed changes
Copy link
Member

@barneycarroll barneycarroll left a comment

Choose a reason for hiding this comment

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

Nice @isiahmeadows

@dead-claudia dead-claudia merged commit a8473e6 into MithrilJS:next Nov 27, 2018
@dead-claudia dead-claudia deleted the hook-order-fix branch November 27, 2018 23:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@StephanHoyer StephanHoyer StephanHoyer approved these changes

@barneycarroll barneycarroll barneycarroll approved these changes

@pygy pygy Awaiting requested review from pygy

Assignees
No one assigned
Labels
Type: Breaking Change For any feature request or suggestion that could reasonably break existing code Type: Bug For bugs and any other unexpected breakage
Projects
Triage/bugs
Status: Closed
Milestone
2.0.0
Development

Successfully merging this pull request may close these issues.

Attrs hooks should always be called *after* component hooks
3 participants
@dead-claudia @StephanHoyer @barneycarroll