Skip to content

chore(deps): replace path-browserify with pathe #5318

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 3 commits into from
Apr 16, 2025

Conversation

OrbisK
Copy link
Contributor

@OrbisK OrbisK commented Apr 14, 2025

Stumbling across #5304 I saw that path-browserify could be replaced by unjs pathe. path-browserify has not been updated for 5 years, is only commonjs and from its docs `path-browserify currently matches the Node.js 10.3 API.

I used pnpm catalog to synchronise versions across the monorepo. We can replace this with the current version if needed.
We can even think about replacing node:path with pathe too.

All vitest tests pass.

If this PR is accepted, I can open a backport to v2.

Copy link

pkg-pr-new bot commented Apr 14, 2025

Open in StackBlitz

vue-component-meta

npm i https://pkg.pr.new/vuejs/language-tools/vue-component-meta@5318

vue-component-type-helpers

npm i https://pkg.pr.new/vuejs/language-tools/vue-component-type-helpers@5318

@vue/language-plugin-pug

npm i https://pkg.pr.new/vuejs/language-tools/@vue/language-plugin-pug@5318

@vue/language-core

npm i https://pkg.pr.new/vuejs/language-tools/@vue/language-core@5318

@vue/language-server

npm i https://pkg.pr.new/vuejs/language-tools/@vue/language-server@5318

@vue/language-service

npm i https://pkg.pr.new/vuejs/language-tools/@vue/language-service@5318

vue-tsc

npm i https://pkg.pr.new/vuejs/language-tools/vue-tsc@5318

@vue/typescript-plugin

npm i https://pkg.pr.new/vuejs/language-tools/@vue/typescript-plugin@5318

commit: e7f8a09

@KazariEX
Copy link
Member

If I remember correctly, directly replacing it with pathe actually results in importing the wrong function type.

@OrbisK
Copy link
Contributor Author

OrbisK commented Apr 16, 2025

If I remember correctly, directly replacing it with pathe actually results in importing the wrong function type.

Results or resulted? 😅

pathe should be comparable to node:path. There may be a mismatch between path-browserify and pathe. Can you still reproduce this bug or show me where i can find it?

@KazariEX
Copy link
Member

image

@OrbisK
Copy link
Contributor Author

OrbisK commented Apr 16, 2025

image

you are right. the * as path is the problem. Let me replace this 😅

@KazariEX
Copy link
Member

I plan to switch to the pnpm catalog in this PR #5174, so let's stick with the original dependency management for now.

@KazariEX KazariEX merged commit 15c7115 into vuejs:master Apr 16, 2025
6 checks passed
@johnsoncodehk
Copy link
Member

johnsoncodehk commented Apr 23, 2025

Thanks for your PR! But I'm going to revert this first, let me explain.

We used upath in the past, and switched to path-browserify for reasons of clarifying module responsibilities: Environment-agnostic modules should not need to consider path normalization.

Since path-browserify only supports POSIX, this made it clear that modules that depend on path-browserify should only accept POSIX format paths. When pathe was introduced, we reintroduced the ambiguity of whether a module might handle win32 format paths.

johnsoncodehk added a commit that referenced this pull request Apr 23, 2025
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.

3 participants