Skip to content

Use global URL constructor #23

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

Merged
merged 1 commit into from
Jan 7, 2022
Merged

Conversation

brendankenny
Copy link
Contributor

@brendankenny brendankenny commented Jan 7, 2022

A much later follow-up to #8 :)

We're still happy robots-parser users over in GoogleChrome/lighthouse. Recently we've been changing how we bundle Lighthouse for use in the browser, and, for whatever reason, the most popular rollup and browserify polyfills for the Node 'url' module don't include the URL property, requiring an extra level of patching to get robots-parser working in the browser.

Node 10 added URL as a global, though, which was released long enough ago that maybe the explicit require() could be dropped and the global used instead?

Thanks!

@samclarke
Copy link
Owner

Thanks for the PR!

I wasn't aware that URL had been added as a global variable. As all currently supported versions of Node include it as a global variable and have for a while now, it would make sense to use it that way, especially if it makes some use cases easier.

@samclarke samclarke merged commit 43c90cb into samclarke:master Jan 7, 2022
@brendankenny
Copy link
Contributor Author

Yes, definitely not essential, but nice to have. Thanks for taking a look so quickly!

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.

2 participants