Skip to content

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

5 participants