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

fix(lib.deno_fetch.d.ts): Extend deprecated fetch() overload #14134

Merged

Conversation

nayeemrmn
Copy link
Collaborator

#14113 broke https://deno.land/x/download@v1.0.1/download.ts. The overload accepting URL should be all-inclusive or it won't accept an argument of type string | URL.

cc @kt3k

Copy link
Member

@lucacasonato lucacasonato left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@lucacasonato lucacasonato merged commit 317f13b into denoland:main Mar 28, 2022
@bebraw
Copy link

bebraw commented Mar 29, 2022

Would it be possible to add a test against this type somehow to avoid breaking it in the future? The regression broke Velociraptor and I imagine many projects depending on it were broken as a result (assuming CI usage here).

@nayeemrmn
Copy link
Collaborator Author

Would it be possible to add a test against this type somehow to avoid breaking it in the future? The regression broke Velociraptor and I imagine many projects depending on it were broken as a result (assuming CI usage here).

Maybe a regression test was deserved but in general there is no end to testing against types that way. It's as (un)likely to happen again as any obscure type def breakage.

@bebraw
Copy link

bebraw commented Mar 29, 2022

Maybe a regression test was deserved but in general there is no end to testing against types that way. It's as (un)likely to happen again as any obscure type def breakage.

Cool, good to know.

I remember seeing dependency oriented testing in Node space. The idea would be to test automatically if an upcoming change breaks dependents but that's a more general thing. I believe having this kind of capability in place would have caught this one, though, and it would probably catch others as well.

@KnorpelSenf
Copy link
Contributor

That would indeed be very cool—before a release is performed, a hand-picked collection of deno.land/x modules is compiled. Is this feasible?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants