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

Prepare a breaking build #76

Merged
merged 10 commits into from Mar 28, 2022
Merged

Prepare a breaking build #76

merged 10 commits into from Mar 28, 2022

Conversation

kanongil
Copy link
Contributor

@kanongil kanongil commented Oct 16, 2021

Contains two breaking changes that removes functionality (#65 & #73), and two breaking changes for maintenance (#74 & #75).

Note that when I removed promise support from emit(), I took care to preserve the behaviour that a handler throw will not stop the other handlers from being called. This is different from how node events work, where a throw immediately falls up the the caller.

Besides the breaking changes, I also took the opportunity to do some cleaning, which can be seen through the remaining commits. For instance I completely removed exports.decorate() which was poorly implemented, not documented, and not used in any hapi projects.

The only outstanding issue before this is ready to be published, is updating any dependencies that are going to have a breaking release for hapijs/hapi#4279. This is pending that they are actually published.

If anyone is up for it, there are some undocumented (and not in ts.d) APIs that would be nice to finally include: The ones I have found are:

  1. exports.validate() (used in hapi, catbox & beam)
  2. the options argument of new Podium() and podium.registerEvent()
  3. podium.once() called without a listener (which then returns a promise).
  4. the podium.few() method.

Additionally, I feel that it would be nice to add a podium.off() method that aliases podium.removeListener().

Let me know if I should break up the first two commits into separate PRs for easier review (but where should it be merged to?).

@kanongil kanongil added the breaking changes Change that can breaking existing code label Oct 16, 2021
@devinivy devinivy changed the base branch from master to v5 March 27, 2022 20:37
Copy link
Member

@devinivy devinivy left a comment

Choose a reason for hiding this comment

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

The work in here looks great. I have peeled-off a v5 branch and will be incorporating this into there alongside some of the other changes that have been documented through issues (primarily thinking of gauge() and off()). Thanks!


static validate(events) {
/** @type {Map<string,internals.EventListener>} */
Copy link
Member

Choose a reason for hiding this comment

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

👍 I am into some well-placed jsdoc type annotations.

@devinivy devinivy marked this pull request as ready for review March 28, 2022 02:43
@devinivy devinivy merged commit d623a9a into hapijs:v5 Mar 28, 2022
@devinivy devinivy added this to the 5.0.0 milestone Mar 28, 2022
@devinivy devinivy self-assigned this Mar 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking changes Change that can breaking existing code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants