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

Reverse hook order for all but `onbeforeupdate` #2297

Merged
merged 2 commits into from Nov 27, 2018

Conversation

3 participants
@isiahmeadows
Copy link
Collaborator

isiahmeadows commented Nov 14, 2018

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

@isiahmeadows isiahmeadows requested a review from barneycarroll Nov 14, 2018

@isiahmeadows isiahmeadows requested a review from pygy as a code owner Nov 14, 2018

@project-bot project-bot bot added this to Needs triage in Triage/bugs Nov 14, 2018

@isiahmeadows isiahmeadows added this to the 2.0.0 milestone Nov 14, 2018

@isiahmeadows isiahmeadows force-pushed the isiahmeadows:hook-order-fix branch from 7e6b665 to e50fe7b Nov 14, 2018

Reverse hook order for all but `onbeforeupdate`
- Drive-by: `onbeforeupdate` prevents subtree redraw if *either* hook
  returns `false`, not *both*.

@isiahmeadows isiahmeadows moved this from Needs triage to Low priority in Triage/bugs Nov 14, 2018

@isiahmeadows isiahmeadows moved this from Low priority to High priority in Triage/bugs Nov 14, 2018

@isiahmeadows

This comment has been minimized.

Copy link
Collaborator

isiahmeadows commented Nov 21, 2018

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

@barneycarroll
Copy link
Member

barneycarroll left a comment

@isiahmeadows isiahmeadows merged commit a8473e6 into MithrilJS:next Nov 27, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

Triage/bugs automation moved this from High priority to Closed Nov 27, 2018

@isiahmeadows isiahmeadows deleted the isiahmeadows:hook-order-fix branch Nov 27, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment