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

refactor!: replace is-promise dependency with instanceof check #137

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jonkoops
Copy link
Contributor

Replaces the is-promise dependency with an instanceof Promise check.

BREAKING: drops support for non-native promises

Closes #136

@jonkoops
Copy link
Contributor Author

This PR should likely not be merged until the next major.

@jonkoops
Copy link
Contributor Author

jonkoops commented Dec 2, 2024

Just ping me whenever a new major is in the works and I will take a moment to rebase the work here.

@wesleytodd
Copy link
Member

Really sorry to have left this one hanging for so long. You are right this is a breaking chance since it would drop support for some of the legacy promise forms that package supports. And looking back at the timing we had already cut the 2.x release line by the time this was opened anyway.

So, what can we do? I think the best option we could do is to check if it is anything other than a native Promise and print a deprecation warning in the 2.x branch. That would set us up to remove this package and fully drop support for anything but native promises in the 3.x line which will ship with express@6. Does that sound like a good next step to you?

@jonkoops
Copy link
Contributor Author

jonkoops commented Mar 5, 2025

Sure, I can go ahead and make a PR that logs a deprecation for this. Do you have any examples of how deprecations should be logged that I could use as inspiration?

@wesleytodd
Copy link
Member

We have used depd for this in the past. This is what I could find in this repo, but we for sure used it in express as well.

@bjohansebas
Copy link
Contributor

I like the idea of showing that warning

@jonkoops
Copy link
Contributor Author

jonkoops commented Mar 6, 2025

Created a PR to add the message under #154

BREAKING: drops support for non-native promises

Closes pillarjs#136

Signed-off-by: Jon Koops <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consider dropping is-promise as a dependency
4 participants