Visitar URL original
Allow URL class object as an argument for fetch() by Maxim-Mazurok · Pull Request #1696 · node-fetch/node-fetch · GitHub
Skip to content

Allow URL class object as an argument for fetch() - #1696

Merged
jimmywarting merged 2 commits into
node-fetch:mainfrom
Maxim-Mazurok:fetch-url
Jan 9, 2023
Merged

Allow URL class object as an argument for fetch()#1696
jimmywarting merged 2 commits into
node-fetch:mainfrom
Maxim-Mazurok:fetch-url

Conversation

@Maxim-Mazurok

@Maxim-Mazurok Maxim-Mazurok commented Jan 5, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

@Maxim-Mazurok Maxim-Mazurok changed the title allow to fetch URL Allow to fetch URL Jan 5, 2023
@Maxim-Mazurok Maxim-Mazurok mentioned this pull request Jan 5, 2023
Closed
@Maxim-Mazurok Maxim-Mazurok changed the title Allow to fetch URL Allow URL class object as an argument for fetch() Jan 5, 2023
LinusU
LinusU reviewed Jan 5, 2023
View reviewed changes
Comment thread @types/index.d.ts Resolved
LinusU
LinusU approved these changes Jan 6, 2023
View reviewed changes

@LinusU LinusU left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍

@Maxim-Mazurok

Maxim-Mazurok commented Jan 7, 2023

Copy link
Copy Markdown
Contributor Author

CI fails for reasons unrelated to the change, it fails with the same errors on the main branch as well.
Following advice from #1657 (comment) adding tsconfig.json with the following contents:

{
	"compilerOptions": {
		"lib": ["DOM"]
	}
}

resolves the issue, however, I'm not sure if that's the correct way to do it.

If it was up to me - I'd force-merge this PR and address the FormData issues separately:

npm run test-types

> node-fetch@3.1.1 test-types
> tsd


  node_modules/formdata-polyfill/esm.min.d.ts:2:10
  ✖    2:10  FormData refers to a value, but is being used as a type here. Did you mean typeof FormData?  
  ✖    3:13  FormData refers to a value, but is being used as a type here. Did you mean typeof FormData?  
  ✖    5:49  FormData refers to a value, but is being used as a type here. Did you mean typeof FormData?  

  @types/index.d.ts:124:3
  ✖  124:3   FormData refers to a value, but is being used as a type here. Did you mean typeof FormData?  
  ✖  137:21  FormData refers to a value, but is being used as a type here. Did you mean typeof FormData?  

  node_modules/fetch-blob/from.d.ts:20:63
  ✖   20:63  File refers to a value, but is being used as a type here. Did you mean typeof File?          
  ✖   25:59  File refers to a value, but is being used as a type here. Did you mean typeof File?          

  node_modules/fetch-blob/file.d.ts:1:75
  ✖    1:75  Element implicitly has an any type because type typeof globalThis has no index signature.    

  8 errors

But happy to add the above mentioned fix if that sounds like a good idea?

@jimmywarting
jimmywarting merged commit e093030 into node-fetch:main Jan 9, 2023
@Maxim-Mazurok
Maxim-Mazurok deleted the fetch-url branch January 15, 2023 02:18
@Maxim-Mazurok

Maxim-Mazurok commented Jan 21, 2023

Copy link
Copy Markdown
Contributor Author

Looking forward to having this published on npm, thanks!

@Maxim-Mazurok

Maxim-Mazurok commented Feb 12, 2023

Copy link
Copy Markdown
Contributor Author

@jimmywarting, I'm not familiar with the release cycle of this library, but I'm still waiting for this to be published in v3, thank you!

@jimmywarting

jimmywarting commented Feb 12, 2023

Copy link
Copy Markdown
Collaborator

Kind of release a new version every time a PR with the commit fix: or feat: is merged. docs and d.ts is rarely never released

@Maxim-Mazurok

Maxim-Mazurok commented Feb 19, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Ah, I wasn't aware of that... This probably should've been a fix:...
Perhaps adding commitizen or some pre-commit hook to validate commit messages would help new contributors?
I've created a dummy PR with fix: to trigger release now: #1716

Maxim-Mazurok added a commit to Maxim-Mazurok/node-fetch that referenced this pull request Feb 19, 2023
@Maxim-Mazurok
fix: release "Allow URL class object as an argument for fetch()" node…
9e5ef99 
…-fetch#1696
@Maxim-Mazurok Maxim-Mazurok mentioned this pull request Feb 19, 2023
Merged
jimmywarting pushed a commit that referenced this pull request Mar 11, 2023
@Maxim-Mazurok
fix: release "Allow URL class object as an argument for fetch()" #1696 (
7b86e94 
#1716)
@github-actions

github-actions Bot commented Mar 11, 2023

Copy link
Copy Markdown

🎉 This PR is included in version 3.3.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@data-sync-user data-sync-user mentioned this pull request Oct 12, 2023
Closed
@ntnguyencse ntnguyencse mentioned this pull request May 19, 2024
Open
This was referenced May 19, 2024
Open
Open
Open
@snyk-io snyk-io Bot mentioned this pull request May 22, 2024
Open
This was referenced May 22, 2024
Open
Open
@jhadev jhadev mentioned this pull request May 24, 2024
Open
@npulsipher4 npulsipher4 mentioned this pull request May 24, 2024
Closed
@mshantanu mshantanu mentioned this pull request Sep 13, 2024
Open
@NOUIY NOUIY mentioned this pull request Sep 13, 2024
Open
This was referenced Sep 14, 2024
Open
Open
Open
Open
Open
Open
Open
This was referenced Sep 21, 2024
Open
Open
Open
@snyk-io snyk-io Bot mentioned this pull request Sep 24, 2024
Open
This was referenced Sep 24, 2024
Open
Open
Open
Open
Open
Open
Open
This was referenced Oct 1, 2024
Open
Open
Open
Open
Open
Open
@ReneMaetzschker ReneMaetzschker mentioned this pull request Oct 26, 2024
Open
@snyk-io snyk-io Bot mentioned this pull request Nov 9, 2024
Closed
@samueljochem samueljochem mentioned this pull request Nov 9, 2024
Open
@mshantanu mshantanu mentioned this pull request Nov 12, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@LinusU LinusU LinusU approved these changes
@jimmywarting jimmywarting jimmywarting approved these changes

Assignees

No one assigned

Labels

released

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

URL is missing in typescript definitions for RequestInfo

3 participants

@Maxim-Mazurok @jimmywarting @LinusU