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

Remove node:path and process.nextTick dependency #117

Open
nitedani opened this issue Aug 14, 2024 · 6 comments · May be fixed by #126
Open

Remove node:path and process.nextTick dependency #117

nitedani opened this issue Aug 14, 2024 · 6 comments · May be fixed by #126
Labels
enhancement ✨ New feature or request

Comments

@nitedani
Copy link

nitedani commented Aug 14, 2024

Remove node:path and process.nextTick dependency to support edge runtimes.
Here is a working example running on vercel edge:
https://hono-react-nitedani-nitedanis-projects.vercel.app/dynamic?_vercel_share=iFkEgX1gF9U1hM806vEBdiywntbfo7CA
All I had to do:

  • replace path.join(a,b) with a + "/" + b - of course this is just for showcase
  • comment out asserts depending on path
  • replace process.nextTick with setTimeout
@brillout
Copy link
Owner

All I had to do:

  • replace path.join(a,b) with a + "/" + b - of course this is just for showcase
  • comment out asserts depending on path
  • replace process.nextTick with setTimeout

Do you still have that diff somewhere?

@samuba
Copy link
Contributor

samuba commented Nov 5, 2024

Second this! Currently cloudflare throws"Error: Dynamic require of \"node:path\" is not supported" during runtime even with node compat mode enabled.

@brillout
Copy link
Owner

brillout commented Nov 5, 2024

@samuba Up for a PR? Should be relatively easy.

@samuba
Copy link
Contributor

samuba commented Nov 5, 2024

@brillout I would be up for it. Would use the path drop in replacement https://github.com/unjs/pathe though, as I see there is more then just path joining involved.
You probably want to keep the runtime code dependency free, so I would drop the 3 file from pathe into the src. What do you think?

@brillout
Copy link
Owner

brillout commented Nov 5, 2024

How about we start with the pathe npm package at first and in a minimal fashion (i.e. only using for the runtime and not for the Vite plugin). Let's then see whether we can remove or trim down pathe.

@samuba samuba linked a pull request Nov 5, 2024 that will close this issue
@samuba
Copy link
Contributor

samuba commented Nov 5, 2024

PR: #126
had to replace usage of path.posix.* with path.* as pathe does not provide that api

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement ✨ New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants