Skip to content

update fun-hooks to latest and update api #3574

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

Merged
merged 8 commits into from
Feb 27, 2019
Merged

Conversation

snapwich
Copy link
Collaborator

@snapwich snapwich commented Feb 21, 2019

Type of change

  • Feature

Description of change

Added a new feature to fun-hooks that allows you to add/remove before and after hooks before the hookable function is actually defined. Added notes on the feature here: https://github.com/snapwich/fun-hooks#naming

Updated Prebid.js to use the new API which replaces the hooks object with a .get method to grab the hookable function. If the hookable function is not found it will create a placeholder for all the before/after hooks until the hookable function is created. It's allows you to "subscribe" to hooks you expect to eventually be present, in a sense. Any hooks that are added using .get but never eventually have a hookable function created for them will throw a warning when .ready is called (because at that point you are saying you expect all hookable functions to be created).

@jsnellbaker check this out and see if it helps with the issue that prompted #3564

Also includes fix from #3544

@snapwich snapwich requested a review from jsnellbaker February 21, 2019 05:16
@snapwich
Copy link
Collaborator Author

Feature was added in: snapwich/fun-hooks#2

@jaiminpanchal27
Copy link
Collaborator

@jsnellbaker Please verify #3574 and update

Copy link
Collaborator

@jsnellbaker jsnellbaker left a comment

Choose a reason for hiding this comment

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

Based on my local tests in trying to setup a hook as described in #3574, it appears to be working with these updates.

@jaiminpanchal27 jaiminpanchal27 merged commit 36f1230 into master Feb 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
LGTM needs 2nd review Core module updates require two approvals from the core team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants