Skip to content

fix: import whatwg-url in a way compatible with ESM Node#1303

Merged
jimmywarting merged 2 commits into2.xfrom
2.x-import-mjs-fix
Sep 22, 2021
Merged

fix: import whatwg-url in a way compatible with ESM Node#1303
jimmywarting merged 2 commits into2.xfrom
2.x-import-mjs-fix

Conversation

@LinusU
Copy link
Member

@LinusU LinusU commented Sep 21, 2021

What is the purpose of this pull request?

  • Documentation update
  • Bug fix
  • New feature
  • Other, please explain:

What changes did you make? (provide an overview)

I imported whatwg-url in a way that is compatible with Node.js running in ESM mode.

Which issue (if any) does this pull request address?

ace7536#r56836783

Is there anything you'd like reviewers to know?

n/a

closes #1305

@jimmywarting
Copy link
Collaborator

jimmywarting commented Sep 21, 2021

Any possible way to unit test this also?
changelog + patch version also?

@LinusU
Copy link
Member Author

LinusU commented Sep 21, 2021

Any possible way to unit test this also?

I haven't found one yet, since I cannot trigger the issue myself (even though I think that you shouldn't be able to import it like that 😄)

Waiting for more information from @robrecord

changelog + patch version also?

Will fix ✅

@robrecord
Copy link

Thank you, I will review ASAP.

@robrecord
Copy link

I have confirmed this fixes the error I was facing. Thank you!

@paranoidjk
Copy link

This should fix #1305

@LinusU
Copy link
Member Author

LinusU commented Sep 22, 2021

@robrecord or @paranoidjk do you know of any easy way we could add a test for this?

@LinusU
Copy link
Member Author

LinusU commented Sep 22, 2021

@jimmywarting I've added changelog entry and bumped the package version. What do you think of merging this without a test for now, and adding one later as this seems to be breaking some peoples workflow and they have to pin node-fetch at a lower version currently?

@jimmywarting
Copy link
Collaborator

Yea, can release it without a test

@jimmywarting jimmywarting merged commit b5417ae into 2.x Sep 22, 2021
@jimmywarting jimmywarting deleted the 2.x-import-mjs-fix branch September 22, 2021 09:16
@robrecord
Copy link

If I can find an easy way to have this tested I will add it here.

@jimmywarting
Copy link
Collaborator

jimmywarting commented Sep 22, 2021

If I can find an easy way to have this tested I will add it here.

Great

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