feat: `node:path` polyfill refinements #7235

marcalexiei · 2025-11-02T05:32:02Z

Problem

Right now the following file pairs have the same content:

packages/json-magic/src/polyfills/index.ts & packages/json-magic/src/polyfills/path.ts

packages/openapi-parser/src/polyfills/index.ts & packages/openapi-parser/src/polyfills/path.ts

Solution

I added a new export on helpers package: @scalar/helpers/node/path.
This exports expose the polyfill that was duplicated in 2 packages.

This polifyll is now used from @scalar/json-magic package.
Regarding @scalar/openapi-parser usage I noticed that was used alongside node:fs so I don't think that the polifyll is necessary.
I'll add a review comment so we can discuss this easily.

Checklist

I've gone through the following:

I've added an explanation why this change is needed.
I've added a changeset (pnpm changeset).
I've added tests for the regression or new feature.
I've updated the documentation (Not needed).

Note

Introduce a shared @scalar/helpers/node/path polyfill, update json-magic to use it, and switch openapi-parser to native node:path, removing duplicated polyfills and adding tests.

Helpers (@scalar/helpers)
- New polyfill: Add @scalar/helpers/node/path with typed POSIX path utilities in src/node/path.ts and export via package.json ("./node/*").
- Tests: Add comprehensive unit tests for path functions (packages/helpers/src/node/path.test.ts).
JSON Magic (@scalar/json-magic)
- Refactor: Replace internal path polyfill with @scalar/helpers/node/path in src/bundle/bundle.ts.
- Cleanup: Remove duplicated polyfill re-exports (src/polyfills/index.ts).
OpenAPI Parser (@scalar/openapi-parser)
- Fix: Use native node:path (dirname, join) in plugins/read-files/read-files.ts.
- Cleanup: Remove duplicated polyfill files (src/polyfills/*).
- Tests: Minor typo fix in read-files.test.ts.

^{Written by Cursor Bugbot for commit 2860d2e. This will update automatically on new commits. Configure here.}

changeset-bot · 2025-11-02T05:32:05Z

🦋 Changeset detected

Latest commit: 2860d2e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 26 packages

Name	Type
@scalar/json-magic	Minor
@scalar/openapi-parser	Patch
@scalar/helpers	Minor
@scalar/api-client	Patch
@scalar/api-reference	Patch
@scalar/oas-utils	Patch
@scalar/workspace-store	Patch
@scalar/fastify-api-reference	Patch
@scalar/import	Patch
@scalar/mock-server	Patch
@scalar/openapi-to-markdown	Patch
@scalar/components	Patch
@scalar/object-utils	Patch
@scalar/postman-to-openapi	Patch
@scalar/sidebar	Patch
@scalar/void-server	Patch
@scalar/nuxt	Patch
@scalar/api-client-react	Patch
scalar-app	Patch
@scalarapi/docker-api-reference	Patch
@scalar/aspire	Patch
@scalar/aspnetcore	Patch
@scalar/webjar	Patch
@scalar/api-reference-react	Patch
@scalar/pre-post-request-scripts	Patch
@scalar/use-codemirror	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

packages/helpers/src/node/path.ts

packages/openapi-parser/src/plugins/read-files/read-files.ts

hanspagel

yeah, maybe we can just use node:path? I don’t remember why we needed a polyfill, but if the build and the tests pass, it’s fine.

marcalexiei · 2025-11-03T10:13:42Z

Hey @hanspagel,

yeah, maybe we can just use node:path? I don’t remember why we needed a polyfill,

I done a quick search:
packages/json-magic/src/bundle/bundle.ts ends up bundled in the standalone file,
so I think that the polifyll was included because some of the code was executed in the browser?
All path functions are used inside resolveReferencePath which might get called when in a browser.

I don’t remember why we needed a polyfill, but if the build and the tests pass, it’s fine.

Ok, I'll update this PR later to see if Ci complains about the change.

hanspagel · 2025-11-03T10:33:23Z

ah yeah, if the relevant code actually ends up in a browser, sure.
I thought it would just be used for the read files plugin or something

thanks for looking into this

marcalexiei · 2025-11-03T11:39:34Z

Adding for completeness:

Using node:path in json-magic bundle causes CI to fail:
https://github.com/scalar/scalar/actions/runs/19031676179/job/54347195263:

ERROR in node:path
Module build failed: UnhandledSchemeError: Reading from "node:path" is not handled by plugins (Unhandled scheme).
Webpack supports "data:" and "file:" URIs by default.
You may need an additional plugin to handle "node:" URIs.

… ponyfills

amritk · 2025-11-03T18:05:01Z

packages/helpers/src/node/path.ts

 export default {
-  extname: extname,
-  basename: basename,
-  dirname: dirname,
-  sep: sep,
-  delimiter: delimiter,
-  relative: relative,
-  join: join,
-  isAbsolute: isAbsolute,
-  normalize: normalize,
-  resolve: resolve,
+  extname,
+  basename,
+  dirname,
+  sep,
+  delimiter,
+  relative,
+  join,
+  isAbsolute,
+  normalize,
+  resolve,
 }


Suggested change

export default {

extname: extname,

basename: basename,

dirname: dirname,

sep: sep,

delimiter: delimiter,

relative: relative,

join: join,

isAbsolute: isAbsolute,

normalize: normalize,

resolve: resolve,

extname,

basename,

dirname,

sep,

delimiter,

relative,

join,

isAbsolute,

normalize,

resolve,

}

export const path = {

extname,

basename,

dirname,

sep,

delimiter,

relative,

join,

isAbsolute,

normalize,

resolve,

}

hey looks good! Can we just avoid the default export? I would have thought biome would have caught this.

Can we just avoid the default export?

I usually avoid default export, however using it would make the module signature different from node:path.
No problem making the change though.

I would have thought biome would have caught this.

I think that is because noDefaultExport rule is not explicitly enabled in the repo.
Consider that recommended rules are disabled:

scalar/biome.json

Line 100 in 8351cf4

"recommended": false,

Here’s a clearer and more polished version of your text, while keeping your original intent and tone:

After finishing the work on enabling knip across the entire repository, I can check which recommended rules can be enabled to improve code consistency and quality of code.
Since you mentioned this, I’d start with enabling noDefaultExport 😅.

hey you are a legend! haha. Lets make the change, no need to follow nodes bad habbits.

we probably didn't turn on recommended because there would be too many infractions 😓, were getting closer though!

hey you are a legend! haha. Lets make the change, no need to follow nodes bad habbits.

Addressed via 2860d2e

amritk

Nice work with the move!

Also @hanspagel there are a few libs that do the same thing if we don't want to maintain this code

https://www.npmjs.com/package/path-browserify
https://www.npmjs.com/package/node-stdlib-browser
https://www.npmjs.com/package/vite-plugin-node-polyfills

geoffgscott · 2025-11-04T11:52:01Z

I would rather us maintain this. Polyfill libraries have been and endless headache in the past. This also encourages a least-usage pattern as you have to roll any polyfills we need.

geoffgscott

Love it.

marcalexiei commented Nov 2, 2025

View reviewed changes

packages/helpers/src/node/path.ts Show resolved Hide resolved

packages/openapi-parser/src/plugins/read-files/read-files.ts Show resolved Hide resolved

marcalexiei marked this pull request as ready for review November 2, 2025 05:42

marcalexiei requested review from DemonHa, amritk, geoffgscott, hanspagel and marclave as code owners November 2, 2025 05:42

hanspagel reviewed Nov 3, 2025

View reviewed changes

marcalexiei closed this Nov 3, 2025

marcalexiei deleted the dedupe-path-polyfill branch November 3, 2025 10:31

marcalexiei restored the dedupe-path-polyfill branch November 3, 2025 10:31

marcalexiei reopened this Nov 3, 2025

marcalexiei added 2 commits November 3, 2025 11:33

feat(helpers): add node:path polyfill

29176d1

feat(json-magic): use @scalar/helpers/node/path polyfill

3b16060

fix(openapi-parser): use node:path instead of polyfill

c6dfaef

marcalexiei force-pushed the dedupe-path-polyfill branch from 420601b to 8345776 Compare November 3, 2025 10:38

This comment was marked as outdated.

Sign in to view

marcalexiei force-pushed the dedupe-path-polyfill branch from 8345776 to c6dfaef Compare November 3, 2025 10:43

This comment was marked as resolved.

Sign in to view

marcalexiei requested a review from hanspagel November 3, 2025 10:50

hanspagel approved these changes Nov 3, 2025

View reviewed changes

fix(helpers): path - use native filter,substring and slice instead of…

eb3199d

… ponyfills

marcalexiei requested a review from hanspagel November 3, 2025 17:24

amritk reviewed Nov 3, 2025

View reviewed changes

amritk approved these changes Nov 3, 2025

View reviewed changes

fix: use named export

2860d2e

geoffgscott approved these changes Nov 4, 2025

View reviewed changes

hanspagel approved these changes Nov 5, 2025

View reviewed changes

hanspagel merged commit c1ecd0c into scalar:main Nov 5, 2025
35 checks passed

hanspagel mentioned this pull request Nov 5, 2025

chore: release #7268

Open

feat: node:path polyfill refinements #7235

feat: node:path polyfill refinements #7235

Conversation

marcalexiei commented Nov 2, 2025 • edited by cursor bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

changeset-bot bot commented Nov 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

Uh oh!

Uh oh!

hanspagel left a comment

Choose a reason for hiding this comment

Uh oh!

marcalexiei commented Nov 3, 2025

Uh oh!

hanspagel commented Nov 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as resolved.

marcalexiei commented Nov 3, 2025

Uh oh!

amritk Nov 3, 2025

Choose a reason for hiding this comment

Uh oh!

marcalexiei Nov 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

amritk Nov 3, 2025

Choose a reason for hiding this comment

Uh oh!

amritk Nov 3, 2025

Choose a reason for hiding this comment

Uh oh!

marcalexiei Nov 3, 2025

Choose a reason for hiding this comment

Uh oh!

amritk left a comment

Choose a reason for hiding this comment

Uh oh!

geoffgscott commented Nov 4, 2025

Uh oh!

geoffgscott left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

feat: `node:path` polyfill refinements #7235

feat: `node:path` polyfill refinements #7235

marcalexiei commented Nov 2, 2025 •

edited by cursor bot

Loading

changeset-bot bot commented Nov 2, 2025 •

edited

Loading

hanspagel commented Nov 3, 2025 •

edited

Loading

marcalexiei Nov 3, 2025 •

edited

Loading