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 type problem with URL and URLSearchParams globals #969

Merged
merged 1 commit into from Dec 7, 2019

Conversation

pmmmwh
Copy link
Contributor

@pmmmwh pmmmwh commented Dec 6, 2019

This fixes #960 , basically duplicating the URL global shim to the types file which is exported to user-land.

@sindresorhus sindresorhus changed the title Duplicate url types to exported modules Fix type problem with URL and URLSearchParams globals Dec 7, 2019
@sindresorhus sindresorhus merged commit 2d5e28d into sindresorhus:master Dec 7, 2019
@pmmmwh pmmmwh deleted the fix/url-types branch December 7, 2019 20:21
@pmmmwh
Copy link
Contributor Author

pmmmwh commented Dec 7, 2019

Shouldn't be https://github.com/sindresorhus/got/blob/master/source/types/url/index.d.ts removed then?

Also should https://github.com/sindresorhus/got/blob/master/source/types/reflect/index.d.ts be moved also? And https://github.com/sindresorhus/got/blob/master/source/types/mimic-response/index.d.ts ?

  1. I kept it because I think the two files signals two different problems, one about URL missing in the global types, and the other about TSC not able to output non module files. If TSC got updated, we could safely remove the change done in this PR, and if URLs got added, we can remove both. If you think this is unnecessary, I can PR to remove the duplication.
  2. They don't have to be added because they are only used internally by the source. Won't bleed type errors into user land.

@szmarczak
Copy link
Collaborator

Honestly I don't know what's the correct solution. Just asking.

@pmmmwh
Copy link
Contributor Author

pmmmwh commented Dec 7, 2019

Hi

Honestly I don't know what's the correct solution. Just asking.

Me neither, they are all workarounds for unfortunate limitations to TS, so I just wanted to make it work and move on ...

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.

Cannot find name 'URLSearchParams'
3 participants