Skip to content

Conversation

@mertcanaltin
Copy link
Member

allows us to process filenames that are already URLs by passing them unchanged

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/node-api

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. node-api Issues and PRs related to the Node-API. labels Jun 4, 2025
@mertcanaltin mertcanaltin force-pushed the mert/fix/filename-url-handling branch from 6b4b308 to 0df3eb0 Compare June 4, 2025 20:09
@codecov
Copy link

codecov bot commented Jun 4, 2025

Codecov Report

❌ Patch coverage is 40.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.14%. Comparing base (641653b) to head (6b50c57).
⚠️ Report is 853 commits behind head on main.

Files with missing lines Patch % Lines
src/node_api.cc 40.00% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #58578      +/-   ##
==========================================
- Coverage   90.21%   90.14%   -0.07%     
==========================================
  Files         635      636       +1     
  Lines      187494   187895     +401     
  Branches    36838    36879      +41     
==========================================
+ Hits       169144   169375     +231     
- Misses      11145    11287     +142     
- Partials     7205     7233      +28     
Files with missing lines Coverage Δ
src/node_api.cc 75.91% <40.00%> (-0.26%) ⬇️

... and 63 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@legendecas legendecas moved this from Need Triage to In Progress in Node-API Team Project Jun 6, 2025
@anonrig anonrig requested a review from addaleax June 8, 2025 01:30
@legendecas legendecas added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 9, 2025
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 9, 2025
@nodejs-github-bot
Copy link
Collaborator

@addaleax addaleax added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jun 12, 2025
@nodejs-github-bot
Copy link
Collaborator

@legendecas
Copy link
Member

The CI failures are relevant:

duration_ms: 212.99
exitcode: 1
severity: fail
stack: |-
  node:assert:92
    throw new AssertionError(obj);
    ^

  AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:
  + actual - expected

  + 'c:\\workspace\\node-test-binary-windows-native-suites\\node\\test\\node-api\\test_general\\build\\Release\\test_general.node'
  - 'file:///c:/workspace/node-test-binary-windows-native-suites/node/test/node-api/test_general/build/Release/test_general.node'

      at Object.<anonymous> (c:\workspace\node-test-binary-windows-native-suites\node\test\node-api\test_general\test.js:18:10)
      at Module._compile (node:internal/modules/cjs/loader:1733:14)
      at Object..js (node:internal/modules/cjs/loader:1898:10)
      at Module.load (node:internal/modules/cjs/loader:1468:32)
      at Module._load (node:internal/modules/cjs/loader:1285:12)
      at TracingChannel.traceSync (node:diagnostics_channel:322:14)
      at wrapModuleLoad (node:internal/modules/cjs/loader:235:24)
      at Module.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:152:5)
      at node:internal/main/run_main_module:33:47 {
    generatedMessage: true,
    code: 'ERR_ASSERTION',
    actual: 'c:\\workspace\\node-test-binary-windows-native-suites\\node\\test\\node-api\\test_general\\build\\Release\\test_general.node',
    expected: 'file:///c:/workspace/node-test-binary-windows-native-suites/node/test/node-api/test_general/build/Release/test_general.node',
    operator: 'strictEqual'
  }

  Node.js v25.0.0-pre

Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mertcanaltin
Copy link
Member Author

mertcanaltin commented Jun 13, 2025

I opened an issue on WHATWG to discuss Windows file path handling in URL parsing

This discussion started from our PR here and ada-url PR ada-url/ada#957.
I wonder what WHATWG thinks about this behavior. whatwg/url#873

lpinca
lpinca previously approved these changes Jun 14, 2025
@lpinca lpinca removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jun 14, 2025
@lpinca lpinca added the blocked PRs that are blocked by other issues or PRs. label Jun 14, 2025
@lpinca
Copy link
Member

lpinca commented Jun 14, 2025

Blocked as per #58578 (comment).

@mertcanaltin
Copy link
Member Author

mertcanaltin commented Dec 21, 2025

I'm waiting, my WHATWG pr jsdom/whatwg-url#304

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

blocked PRs that are blocked by other issues or PRs. c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. node-api Issues and PRs related to the Node-API.

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

8 participants