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

Support browser #21

Open
2 tasks done
zhmushan opened this issue Jan 21, 2022 · 18 comments
Open
2 tasks done

Support browser #21

zhmushan opened this issue Jan 21, 2022 · 18 comments
Labels
good first issue Good for newcomers

Comments

@zhmushan
Copy link
Contributor

Prerequisites

  • I have written a descriptive issue title
  • I have searched existing issues to ensure the feature has not already been requested

🚀 Feature Proposal

Are there plans to support browsers? It looks like it can be done with just any compiler tool.

Motivation

No response

Example

No response

@mcollina
Copy link
Member

Thanks for reporting! Our first focus was to improve the cold start of Fastify applications. Supporting browsers would be amazing too! Would you like to send a PR?

@zekth
Copy link
Member

zekth commented Jan 21, 2022

We made this tool to be usable in the browser too but we did not manage to create a browser test stack yet. If you're willing to help on this task that would be great

@zhmushan
Copy link
Contributor Author

It's a bit unfortunate that the performance of parse will drop a lot if URL.domainToASCII is replaced (this function is actually implemented in C)😅

@zekth
Copy link
Member

zekth commented Jan 21, 2022

It's a bit unfortunate that the performance of parse will drop a lot if URL.domainToASCII is replaced (this function is actually implemented in C)😅

Will it be replaced directly by the compiler tool?

@Uzlopak
Copy link
Contributor

Uzlopak commented Jun 22, 2022

For automated browser tests we would need e.g. saucelabs or browserstack and run the tests with karma. saucelabs costs atleast 149 $/month.

@mcollina
Copy link
Member

Checkout the setup we have for readable-stream: https://github.com/nodejs/readable-stream

It uses playwright and github actions

@zekth
Copy link
Member

zekth commented Jul 30, 2023

For automated browser tests we would need e.g. saucelabs or browserstack and run the tests with karma. saucelabs costs atleast 149 $/month.

can't we just build a webpage and run it and use Cypress to test the different cases?

@mcollina
Copy link
Member

sure thing!

@Uzlopak
Copy link
Contributor

Uzlopak commented Jul 30, 2023

I kind of dont like cypress anymore. Its too big for what it has to do.

If it would be my package, I would

  • replace tap with tape
  • put all the code into one file, basically moving everything into the index.js to avoid bundling
  • if global has URL dont require it it, because we are in a browser environment
  • run the tests with playwright-test, except the compatibility.test.js
  • use c8 for coverage

@mcollina
Copy link
Member

@Uzlopak this module is deliberately different from the URL global as it fulfill a different niche requirement.

@Uzlopak
Copy link
Contributor

Uzlopak commented Jul 30, 2023

Then we need to add an external punycode library or incorporate a fast punycode implementation, to run in the browser

@mcollina
Copy link
Member

Sure, allowing this to run in the browser is a lot of work. I don't have a need for it but I would be happy to review a PR.

@voxpelli
Copy link

voxpelli commented Jun 4, 2024

For reference, ajv tried using this library, but since this library relies on node:url they accidentally broke browser compatibility: ajv-validator/ajv#2415

So if anyone wants to collaborate with them or suggest a good path of action, then that would probably be welcome

@jasoniangreen
Copy link

Hi there! I'm helping out with AJV and I'd be happy to collaborate. We'd be very happy to switch AJV to fast-uri if we can figure out these issues. While it did seem that the issues were mostly related to breaking browser builds, there was one issue that claimed to be a regression ajv-validator/ajv#2447. But one step at a time, first step is browser compatibility.

@gurgunday
Copy link
Member

Hey, I think we can easily make this happen

#73 could cause some issues in terms of 100% compatibility, but I don't think it's a huge problem, and like you said, one step at a time ;)

I'll take a look at this

@gurgunday
Copy link
Member

After #83, https://cdn.jsdelivr.net/npm/fast-uri/+esm seems to work for me in the browser

@gurgunday
Copy link
Member

But I guess we need a proper dist for the web

@mcollina
Copy link
Member

mcollina commented Jun 8, 2024

The typical usage is bundled & running in a browser env. So we need a playwright job that verifies this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

7 participants