Reviewing files that changed from the base of the PR and between d05d8ef and 2c7269c.

• yarn.lock is excluded by !**/yarn.lock, !**/*.lock

• .editorconfig (1 hunks)
• .github/workflows/autofix.yml (1 hunks)
• .github/workflows/pkg-pr-new.yml (1 hunks)
• .github/workflows/release.yml (2 hunks)
• .github/workflows/size-limit.yml (1 hunks)
• .size-limit.json (1 hunks)
• README.md (1 hunks)
• benchmarks/benchmark.cjs (1 hunks)
• package.json (6 hunks)
• src/common.ts (1 hunks)
• src/constants.ts (1 hunks)
• src/helpers.ts (1 hunks)
• src/index.ts (2 hunks)
• src/types.ts (2 hunks)
• tsconfig.json (1 hunks)

• GitHub Check: Codacy Static Code Analysis
• GitHub Check: Lint and Test with Node.js 23 on macos-latest
• GitHub Check: Lint and Test with Node.js 22 on windows-latest
• GitHub Check: Lint and Test with Node.js 22 on macos-latest
• GitHub Check: Lint and Test with Node.js 20 on windows-latest
• GitHub Check: Lint and Test with Node.js 18 on macos-latest
• GitHub Check: Lint and Test with Node.js 18.18 on windows-latest
• GitHub Check: Lint and Test with Node.js 22 on windows-latest
• GitHub Check: Lint and Test with Node.js 23 on windows-latest
• GitHub Check: Lint and Test with Node.js 20 on windows-latest
• GitHub Check: Lint and Test with Node.js 18.18 on windows-latest
• GitHub Check: Lint and Test with Node.js 18 on windows-latest
• GitHub Check: Lint and Test with Node.js 18 on macos-latest

tsconfig.json (1)

4-4:

Details

❓ Verification inconclusive

Good addition of isolatedModules flag.

Adding "isolatedModules": true is beneficial for ensuring compatibility with single-file transpilers (like esbuild, SWC, or Babel) that don't have access to the full TypeScript type-checking context. This setting enforces stricter module boundaries and helps catch potential issues early.

Run the following script to verify that the codebase complies with isolatedModules restrictions:

---

🏁 Script executed:

#!/bin/bash
# Description: Check for potential issues with isolatedModules 

# 1. Verify no non-module files (files without imports/exports)
echo "Checking for non-module files that might cause issues with isolatedModules:"
find src -name "*.ts" -exec grep -L -E "import|export" {} \;

# 2. Check for const enum usage which isn't supported with isolatedModules
echo "Checking for const enum usage (not supported with isolatedModules):"
rg -l "const enum" src

Length of output: 396

---

IsolatedModules Setting Verified – Manual Verification Advised

The addition of "isolatedModules": true to tsconfig.json is a strong improvement. Automated checks (using the provided script) did not report any TypeScript files lacking module declarations (i.e., missing import/export statements) nor any usage of const enum. This indicates that the codebase appears to comply with the restrictions imposed by isolatedModules.

However, the absence of detailed output (i.e., an empty result) means that the verification relies solely on the lack of error messages. To ensure there are no overlooked edge cases, please perform a manual verification in your CI or local environment.

• File: tsconfig.json (Line 4) — "isolatedModules": true
• Checks Performed:
  • No non-module .ts files found in src (all have at least one import or export).
  • No occurrences of const enum detected in src.

If manual verification confirms these findings, this review comment can be considered resolved.

benchmarks/benchmark.cjs (1)

10-10: Improved return type annotation accuracy.

The updated JSDoc return type is more accurate, as the perfCase function can indeed return either a PerfResult or void (when an error occurs while requiring a module and the function returns early).

README.md (2)

207-209: Clean presentation of Open Collective sponsors.

The simplified presentation format for the Open Collective sponsors improves readability while maintaining the same information.

---

213-215: Clean presentation of Open Collective backers.

Similar to the sponsors section, this simplification of the backers section enhances readability without losing any information.

🧰 Tools

🪛 LanguageTool

[style] ~215-~215: Using many exclamation marks might seem excessive (in this case: 11 exclamation marks for a text that’s 6648 characters long)
Context: ...----------------------------------- | | | | [![UnTS Open Collective backers](https://o...

(EN_EXCESSIVE_EXCLAMATION)

package.json (5)

6-8: Good updates to package metadata.

The changes improve package metadata consistency:

• Repository URL now uses a cleaner format without the git+ prefix
• Author field follows a more standard format with the email before the URL
• Funding URL now points specifically to synckit's Open Collective rather than the more general UnTS collective

---

57-57: Good addition of format script.

Adding a dedicated format script using Prettier improves code consistency and aligns with the PR objective of refactoring and improving code organization.

---

125-125: Improved Jest module mapping.

The moduleNameMapper configuration now points specifically to the index.ts file rather than the entire src directory, which provides a more precise mapping for the synckit module in tests.

---

131-132: Simplified ts-jest configuration.

The ts-jest transformer configuration has been simplified to only include the essential useESM: true setting, removing potentially redundant tsconfig options.

---

21-24:

Details

❓ Verification inconclusive

Verify the need for the new module-sync export.

A new "module-sync" export configuration has been added that duplicates the "import" configuration. Please confirm this is intentional and necessary.

---

🏁 Script executed:

#!/bin/bash
# Description: Check if any code uses the new module-sync export condition

# Look for any references to module-sync in the codebase
echo "Searching for module-sync references in the codebase:"
rg "module-sync" --glob "!package.json"

# Check for any import patterns that might be using this condition
echo "Checking Node.js condition exports documentation:"
echo "The 'module-sync' is not a standard Node.js export condition. For reference, standard conditions include: 'import', 'require', 'node', 'default', etc."

Length of output: 606

---

Action Required: Confirm and Document the Usage of "module-sync"

Based on our review:

• Our search did not identify any in-code usage of the "module-sync" condition.
• Note that "module-sync" is not part of the official Node.js export condition list (which includes conditions like "import", "require", "node", and "default").

Please verify if duplicating the "import" configuration under a custom "module-sync" key is intentional. If this export condition is meant to serve a specific purpose, consider adding documentation or comments in the configuration to clarify its intent.

.size-limit.json (1)

1-6: Good addition of size limits

Setting up size limits for the project is a good practice to prevent bundle bloat. The 1kB limit for the main JavaScript file aligns well with the objective of splitting code into chunk files and maintaining a lean bundle size.

.editorconfig (1)

1-10: Style adjustment looks good

The removal of spaces around equal signs is a consistent formatting change that makes the file more compact. This change is purely stylistic and doesn't affect functionality.

.github/workflows/pkg-pr-new.yml (1)

30-30: Command enhancement with --compact flag

Adding the --compact flag to the publish command is a good enhancement that will likely make the publish output more concise and readable.

src/common.ts (1)

3-4: Implementation of hasFlag looks good

The hasFlag utility correctly checks both NODE_OPTIONS and process.argv, providing a comprehensive way to detect command-line flags.

.github/workflows/release.yml (2)

12-16: Enhanced workflow permissions for secure releases

These permission declarations follow security best practices by explicitly defining the required permission scopes rather than using broad access. The permissions are appropriate for a release workflow that needs to modify content, create authentication tokens, and manage pull requests.

---

50-50: Added npm provenance for package security

Enabling npm provenance is an excellent security practice as it creates verifiable links between packages and their source code/build instructions, enhancing supply chain security.

src/types.ts (2)

1-3: Improved imports for better type safety

The updated imports provide necessary types for the new interface and establish a clear relationship with the constants file.

---

84-90:

Details

✅ Verification successful

Well-structured options interface for better API design

The new SynckitOptions interface provides a clean, type-safe way to configure the library. This aligns well with the PR's refactoring goal by improving code organization and maintainability.

---

🏁 Script executed:

#!/bin/bash
# Verify how this new interface is being used across the codebase

# Find files that might use SynckitOptions
echo "Files that likely use SynckitOptions:"
rg --type ts "SynckitOptions" -l

# Check potential usage patterns
echo -e "\nPotential usage patterns:"
rg --type ts "SynckitOptions" -A 5 -B 5

Length of output: 2291

---

Interface usage verified—approve changes.

• The SynckitOptions interface is properly defined in src/types.ts and is being used consistently in both src/index.ts and src/helpers.ts.
• Usage patterns across the codebase confirm that the type-safe configuration aligns with the PR's refactoring goals and improves maintainability.

.github/workflows/autofix.yml (1)

1-37: Well-implemented autofix workflow with strong security practices

This workflow implements automatic code formatting and fixes for pull requests, which helps maintain code quality and consistency. I particularly appreciate the use of pinned commit hashes for actions rather than just version tags, which is an excellent security practice to prevent supply chain attacks.

src/index.ts (4)

3-3: Import usage looks good.
The new import of fileURLToPath neatly addresses file URL conversions later in the code.

---

10-16: New imports from local modules are well-integrated.
The functions and constants (compareVersion, NODE_VERSION, etc.) appear consistently referenced below.

---

23-23: Appropriate addition of SynckitOptions import.
It aligns with the createSyncFn function's second parameter, ensuring type safety.

---

28-30:

Details

✅ Verification successful

Re-exporting modules: check for naming collisions.
Re-exporting all items from common.js, constants.js, and helpers.js is convenient but may risk collisions if multiple modules share export names.

---

🏁 Script executed:

#!/bin/bash
# Verify no conflicting re-exports. We'll search for duplicated named exports in the local modules.
echo "Searching for named exports in common.js, constants.js, and helpers.js..."
rg -A2 --type ts --json "export (const|function|class) " 

Length of output: 22020

---

Re-export modules: confirmed no naming collisions detected.

After reviewing the exported symbols in the local modules, we verified that the exports from common.js, constants.js, and helpers.js are unique. There is no overlap between the exported names from these modules, so the risk of naming collisions appears minimal with the current state of the code.

src/constants.ts (4)

1-2: Imports from common.js and type definitions.
The usage of compareVersion and hasFlag should be compatible with lines below.

---

4-21: Well-structured enumeration of TypeScript runners.
Using as const along with ValueOf<typeof TsRunner> is a neat pattern for typed enum-like objects.

---

31-59: Extensive Node version constants appear valid.
They reflect the documented Node.js features. Keep them updated as new Node releases surface.

---

60-82: Defaults for timeouts, exec arguments, shims, and runner.
Logic for deriving values from environment variables is straightforward.

src/helpers.ts (10)

13-33: Local imports from common.js, constants.js, and types.js properly centralize logic.
This helps unify version checks, flags, and type definitions.

---

45-52: isFile function is straightforward and handles exceptions gracefully.
Synchronous implementation is fine in low-volume contexts.

---

54-56: dataUrl function is concise and correct.
Using encodeURIComponent is recommended to avoid special character issues.

---

266-269: md5Hash is acceptable for non-security contexts.
Keep in mind MD5 is not collision-resistant enough for cryptographic security but fine for caching or quick checks.

---

320-331: _generateGlobals reduces repeated code for shim imports.
Straightforward approach—no concerns.

---

341-383: generateGlobals caches references in .synckit folder.
MD5 collisions are unlikely, but it's worth noting the edge case. Overall, this is a clever method to manage dynamic imports.

---

385-399: Reintroduced extractProperties for preserving custom error fields.
Good approach—no concerns.

---

1-12:

Details

❓ Verification inconclusive

Initial imports appear standard.
Includes Node built-ins and references to @pkgr/core. Confirm that @pkgr/core is included and correctly licensed in your project.

---

🏁 Script executed:

#!/bin/bash
# Confirm @pkgr/core usage in package.json
rg '"@pkgr/core"' package.json

Length of output: 60

---

@pkgr/core Dependency Verification

• The dependency is confirmed in your package.json as "@pkgr/core": "^0.2.0", so its inclusion is correct.
• Please also verify that the license of @pkgr/core complies with the project’s requirements.

---

576-596:

Details

❓ Verification inconclusive

overrideStdio hooks into internal _writev.
Such patches can break on future Node versions if _writev changes. Confirm it remains stable.

---

🌐 Web query:

Is `process.stdout._writev` stable across Node releases?

💡 Result:

The stability of process.stdout._writev across Node.js releases depends on its documentation and implementation details. Analyzing Node.js documentation and related resources:

1. Underscored Methods Convention:

   • Methods prefixed with underscores (like _writev) are typically considered internal/private APIs in Node.js[1][3][5]. These are not part of the public API surface and aren't subject to semantic versioning guarantees.

2. Writev Functionality Context:

   • The _writev method implements batch writing for stream implementations, but this is an internal detail of Writable streams rather than a public method[1][8]
   • Node.js documentation makes no explicit stability guarantees for process.stdout._writev in any version reviewed (v0.8.7 to v23.10.0)[1][2][3][9]

3. Recommended Alternatives:

   // Use public API methods
   process.stdout.write(chunk, encoding, callback)

   • Public stream methods like write() are explicitly documented as stable[1][3][5]
   • For batch writes, consider using Writeable stream implementations with proper backpressure handling[8]

Stability Implications:

	Public API	Underscored Methods
Version Guarantees	Semver protection	No guarantees
Documentation	Explicitly described	Internal details
Breaking Changes	Major version only	Any release

The Node.js documentation explicitly states: "Applications should not use internal methods"[3]. While _writev exists in current implementations, its stability cannot be guaranteed across releases. For production code, use only documented stream methods from the public API[1][3][5][8].

Citations:

• 1: https://nodejs.org/api/child_process.html
• 2: https://nodejs.org/download/release/v0.8.7/docs/api/process.html
• 3: https://nodejs.org/api/process.html
• 4: https://blog.logrocket.com/using-stdout-stdin-stderr-node-js/
• 5: https://nodejs.org/download/release/v6.10.2/docs/api/console.html
• 6: https://www.npmjs.com/package/cli-color
• 7: https://github.com/orgs/nodejs/discussions/43568
• 8: https://blog.platformatic.dev/a-guide-to-reading-and-writing-nodejs-streams
• 9: https://nodejs.org/download/release/v6.15.1/docs/api/process.html
• 10: nodejs/help#569
• 11: https://bun.sh/guides/write-file/stdout

---

Caution: Overriding an Internal Node.js API
Your implementation of overrideStdio overrides process.stdout._writev (and _writev for stderr), which is an internal, underscored method without stability guarantees. According to Node.js guidelines, such internal APIs can change without notice in future releases. Even though this behavior currently works, it poses a risk as future Node versions are not bound by semantic versioning for internal methods.

Recommendations:

• Consider switching to supported, public stream methods (e.g., using process.stdout.write) if possible.
• If overriding _writev remains necessary, ensure robust tests and feature detection are in place to catch any breaking changes in upcoming Node releases.

---

270-318:

Details

✅ Verification successful

encodeImportModule is flexible but watch out for code injection.
If any portion of moduleNameOrGlobalShim can be user-controlled, sanitize carefully.

---

🏁 Script executed:

#!/bin/bash
# Searching for external calls that pass user-provided input to encodeImportModule
rg --json "encodeImportModule"

Length of output: 5849

---

Attention: Verify Input Sanitization in encodeImportModule

Our review of the usage of encodeImportModule in the codebase shows that it is currently invoked only with hardcoded or internally trusted values (as observed in the test files). While no instance of unsanitized, user-controlled input was detected, the function does not perform any sanitization on its inputs. Please ensure that if in the future any part of the moduleNameOrGlobalShim argument (or its inner properties like moduleName or globalName) might come from an untrusted source, proper sanitization and validation measures are implemented to prevent potential code injection vulnerabilities.

• File: src/helpers.ts (function encodeImportModule)
• Pages to consider: Ensure all calls to encodeImportModule (e.g., in test/utils.spec.ts) do not expose the function to user-controlled input in production scenarios.

Overall, the current implementation is acceptable given its present usage, so no immediate changes are required.

Reviewing files that changed from the base of the PR and between 2c7269c and 4cfe2e9.

• yarn.lock is excluded by !**/yarn.lock, !**/*.lock

• .editorconfig (1 hunks)
• .github/workflows/autofix.yml (1 hunks)
• .github/workflows/ci.yml (1 hunks)
• .github/workflows/pkg-pr-new.yml (1 hunks)
• .github/workflows/release.yml (2 hunks)
• .github/workflows/size-limit.yml (1 hunks)
• .size-limit.json (1 hunks)
• README.md (1 hunks)
• benchmarks/benchmark.cjs (1 hunks)
• package.json (6 hunks)
• src/common.ts (1 hunks)
• src/constants.ts (1 hunks)
• src/helpers.ts (1 hunks)
• src/index.ts (2 hunks)
• src/types.ts (2 hunks)
• tsconfig.json (1 hunks)

• .github/workflows/ci.yml

• .size-limit.json
• .editorconfig
• benchmarks/benchmark.cjs
• .github/workflows/pkg-pr-new.yml
• tsconfig.json
• .github/workflows/autofix.yml
• .github/workflows/size-limit.yml
• .github/workflows/release.yml
• src/common.ts
• src/types.ts
• package.json

• GitHub Check: Lint and Test with Node.js 22 on macos-latest
• GitHub Check: Lint and Test with Node.js 20 on macos-latest
• GitHub Check: Lint and Test with Node.js 22 on windows-latest
• GitHub Check: Lint and Test with Node.js 20 on windows-latest
• GitHub Check: Lint and Test with Node.js 18 on macos-latest
• GitHub Check: Lint and Test with Node.js 23 on macos-latest
• GitHub Check: Lint and Test with Node.js 23 on windows-latest
• GitHub Check: Lint and Test with Node.js 22 on windows-latest
• GitHub Check: Lint and Test with Node.js 20 on macos-latest
• GitHub Check: Lint and Test with Node.js 18 on windows-latest
• GitHub Check: Codacy Static Code Analysis

README.md (1)

213-215: Review of Backers Section Table Format

The revisions in the Backers section correctly update the labeling to "Open Collective backers" and maintain a consistent table layout. This clear distinction from the Sponsors section improves clarity regarding funding sources.

🧰 Tools

🪛 LanguageTool

[style] ~215-~215: Using many exclamation marks might seem excessive (in this case: 11 exclamation marks for a text that’s 6648 characters long)
Context: ...----------------------------------- | | | | [![UnTS Open Collective backers](https://o...

(EN_EXCESSIVE_EXCLAMATION)

src/index.ts (6)

3-3: Importing fileURLToPath is correct and straightforward.
No issues found with the usage here.

---

10-10: New import for version comparison looks good.
Usage of compareVersion is relevant to node version checks later in the file.

---

11-11: Re-introducing Node version constants.
These constants will help manage feature checks in worker threads.

---

12-16: Imports from helpers are well-scoped.
All three functions (extractProperties, overrideStdio, startWorkerThread) are subsequently used and correctly imported.

---

23-23: Exporting SynckitOptions is beneficial for public-facing types.
No issues with the addition.

---

78-78:

Details

✅ Verification successful

Ensure monkey patching process[type]._writev does not cause side effects.
Overriding _writev captures worker logs, but be cautious when other modules may rely on default _writev.

Would you like to verify that no other libraries are interfering with this override?

---

🏁 Script executed:

#!/bin/bash
rg '_writev' -A 3 -B 3

Length of output: 498

---

Override _writev Safety Confirmed

After running the search, we verified that the only modification of process[type]._writev is performed in our own src/helpers.ts helper. No other instances in the codebase indicate interference from third-party libraries or alternative overrides. While the current monkey patch appears safe, please ensure comprehensive testing and proper documentation so that future maintainers are aware of the override’s potential impact if any module ever relies on the original behavior.

• Location: src/helpers.ts (implementation of the monkey patch)
• Action: Document the override in project documentation or inline comments and add tests to cover any edge cases involving _writev.

src/constants.ts (14)

1-3: Imports for compareVersion and hasFlag plus type references.
All set for environment-based logic and type usage.

---

4-19: Defining TsRunner constants comprehensively.
This neatly enumerates all supported TypeScript runtimes.

---

21-22: Using ValueOf for TsRunner is a neat approach.
Generates a union type from the constant object.

---

23-29: Destructuring environment variables with defaults.
Works fine, but be cautious about usage of ! for SYNCKIT_GLOBAL_SHIMS in later code.

Please confirm no runtime issues if SYNCKIT_GLOBAL_SHIMS is undefined, as it’s used non-nullably.

---

31-32: Defining minimum Node versions for specific TS features.
Straightforward numeric strings. No issues found.

---

34-45: Node version-based feature flags for TS usage.
Helpful to track shipping features across Node releases.

---

46-49: Explicit flags for controlling type stripping transforms.
Clear and easy to reference.

---

50-53: Splitting and exporting NODE_OPTIONS plus storing NODE_VERSION.
Enables checks and manipulations of node flags.

---

54-59: NO_STRIP_TYPES logic with fallback checks.
Smart usage of comparisons and environment flags. Just be sure process.features.typescript is defined in all Node versions in use.

---

60-61: Default timeout from SYNCKIT_TIMEOUT.
No issues. Numeric cast is straightforward.

---

62-65: Defaults for execArgv and tsRunner.
Simplifies standard usage under typical environment settings.

---

66-69: Boolean check for DEFAULT_GLOBAL_SHIMS.
Similar caution as above: verify that SYNCKIT_GLOBAL_SHIMS is truly set or handle undefined gracefully.

---

70-80: Creating default global shims preset.
Specifically includes fetch and performance from Node. Good introduction for an out-of-box polyfill.

---

82-82: Constant for INT32_BYTES.
Self-explanatory for usage with shared memory.

src/helpers.ts (11)

1-12: Imports for file system, crypto, and worker threads.
No concerns; references appear valid for subsequent usage.

---

13-33: Gathering config and type references from local modules.
All used for advanced worker logic and environment checks.

---

45-52: isFile function with safe error handling.
Catches potential ENOENT or permission errors gracefully.

---

54-56: dataUrl function to generate JavaScript data URLs.
Compact and effective for passing inline code to Worker.

---

266-269: md5Hash function usage is acceptable for caching.
MD5 is not secure for crypto, but sufficient for content checks or naming.

---

270-318: encodeImportModule forms dynamic imports or requires.
Implementation handles absolute vs. relative paths well. Good job accounting for Windows path escapes.

---

320-331: _generateGlobals utility
Builds a chain of import statements or require calls for shims. Implementation is straightforward.

---

332-383: generateGlobals includes caching and writes .mjs for ESM usage.
Logical approach to storing and reusing global shim injection. Be mindful of potential concurrency if multiple workers attempt to create the same file.

---

385-398: extractProperties for transferring error metadata.
Helps preserve stack and custom fields. Straightforward property copying.

---

576-596: overrideStdio monkey patches _writev.
Capturing I/O is likely intentional, but watch for side effects if the process’s default I/O streaming changes mid-execution.

---

400-575:

Details

❓ Verification inconclusive

startWorkerThread sets up comprehensive worker logic.
Overall logic is sound: spins up a worker, handles ES modules vs. CommonJS, and supports TS transformations. Be mindful that sharedBuffer must remain consistent if multiple threads are spawned simultaneously.

To confirm concurrency safety with single sharedBuffer usage, consider verifying that multiple concurrent calls do not overwrite each other:

---

🏁 Script executed:

#!/bin/bash
rg "startWorkerThread" -A 6 -B 4

Length of output: 1413

---

Verify Concurrency Safety with Shared Buffer Handling

The implementation of startWorkerThread is robust overall—managing ES modules versus CommonJS, handling various TS runners, and integrating TS transformations appropriately. However, please ensure that the module-scoped sharedBuffer and sharedBufferView remain concurrency-safe when multiple worker threads are spawned. In their current form, these shared globals are initialized once and reused, so confirm that simultaneous invocations do not lead to unintended overwrites or race conditions. You might consider additional tests or safeguards to isolate shared state per worker if necessary.

• Verify concurrent calls to startWorkerThread to ensure that the shared buffer does not become a contention point.
• Consider the implications of using a single shared buffer when scaling to multiple threads.

Reviewing files that changed from the base of the PR and between 4cfe2e9 and 16fb628.

• src/helpers.ts (1 hunks)
• src/index.ts (3 hunks)

• GitHub Check: Codacy Static Code Analysis
• GitHub Check: Lint and Test with Node.js 23 on windows-latest
• GitHub Check: Lint and Test with Node.js 23 on macos-latest
• GitHub Check: Lint and Test with Node.js 18.18 on windows-latest
• GitHub Check: Lint and Test with Node.js 22 on macos-latest
• GitHub Check: Lint and Test with Node.js 18.18 on macos-latest
• GitHub Check: Lint and Test with Node.js 20 on windows-latest
• GitHub Check: Lint and Test with Node.js 22 on windows-latest
• GitHub Check: Lint and Test with Node.js 18 on windows-latest
• GitHub Check: Lint and Test with Node.js 23 on macos-latest
• GitHub Check: Lint and Test with Node.js 23 on windows-latest
• GitHub Check: Lint and Test with Node.js 22 on windows-latest
• GitHub Check: Lint and Test with Node.js 18.18 on windows-latest
• GitHub Check: Lint and Test with Node.js 20 on windows-latest
• GitHub Check: Lint and Test with Node.js 18 on macos-latest
• GitHub Check: Lint and Test with Node.js 18 on windows-latest

src/index.ts (9)

3-3: No concerns with the new import usage.
The fileURLToPath utility appears to be used correctly to handle path conversions.

---

10-11: Imports are properly aligned with usage.
The newly imported compareVersion, NODE_VERSION, and LOADER_SUPPORTED_NODE_VERSION constants/functions are used consistently below. No issues.

---

12-12: Refactored imports look good.
Bringing extractProperties, overrideStdio, and startWorkerThread into this file from helpers is consistent with the new structure.

---

23-23: SynckitOptions import is correct.
This type is used in the createSyncFn function and ensures clarity of function parameters.

---

28-30: Re-exporting modules.
Re-exporting from common.js, constants.js, and helpers.js broadens the public API. Ensure these exports are intended for external usage.

---

35-47: Enhanced JSDoc for createSyncFn.
These doc changes accurately describe the function’s purpose, parameters, and usage, improving maintainability.

---

80-80: Coverage ignore is fine.
Since this line only affects coverage, no further action is needed unless coverage of the next block is desired.

---

82-91: Good improvement in function documentation.
Documenting runAsWorker clarifies the logic and helps maintain the code more easily.

---

103-103: Potential concurrency consideration for I/O override.
overrideStdio(stdio) wraps the global process.stdout and process.stderr in a worker context. If multiple workers attempt this override, collisions may occur. Please verify if this usage pattern suits your concurrency needs.

src/helpers.ts (7)

1-52: Initial imports and isFile utility look good.
Handling fs.statSync exceptions by returning false is a safe fallback for non-existent files.

---

54-56: Simple utility for data URLs.
dataUrl provides a concise implementation. No issues noted.

---

266-269: MD5 hashing logic is fine.
Uses the Node crypto module correctly, returning a hex digest. No concerns.

---

270-318: Be cautious with dynamic module imports.
encodeImportModule constructs import/require statements on the fly. If moduleNameOrGlobalShim is user-supplied, consider validating inputs to avoid arbitrary code injections.

---

385-405: extractProperties utility is straightforward.
Shallow-copying properties from error objects helps preserve debugging info across worker boundaries.

---

407-505: Shared buffer handling is clear.
Uses an Int32Array for synchronization. This approach is appropriate for the intended blocking mechanism.

🧰 Tools

🪛 GitHub Check: codecov/patch

[warning] 462-462: src/helpers.ts#L462
Added line #L462 was not covered by tests

---

[warning] 464-464: src/helpers.ts#L464
Added line #L464 was not covered by tests

---

[warning] 479-479: src/helpers.ts#L479
Added line #L479 was not covered by tests

---

605-624: Reimplementation of overrideStdio.
Overriding _writev is useful for capturing I/O from worker threads. As noted before, concurrency must be managed carefully if multiple workers run at once.

🧰 Tools

🪛 GitHub Check: codecov/patch

[warning] 607-609: src/helpers.ts#L607-L609
Added lines #L607 - L609 were not covered by tests

---

[warning] 614-614: src/helpers.ts#L614
Added line #L614 was not covered by tests

---

[warning] 621-621: src/helpers.ts#L621
Added line #L621 was not covered by tests

Reviewing files that changed from the base of the PR and between 16fb628 and 8c215d4.

• yarn.lock is excluded by !**/yarn.lock, !**/*.lock

• package.json (6 hunks)
• src/constants.ts (1 hunks)
• test/common.spec.ts (1 hunks)

• package.json

• GitHub Check: Lint and Test with Node.js 23 on macos-latest
• GitHub Check: Lint and Test with Node.js 22 on windows-latest
• GitHub Check: Lint and Test with Node.js 23 on windows-latest
• GitHub Check: Lint and Test with Node.js 20 on macos-latest
• GitHub Check: Lint and Test with Node.js 20 on windows-latest
• GitHub Check: Lint and Test with Node.js 22 on macos-latest
• GitHub Check: Lint and Test with Node.js 18.18 on windows-latest
• GitHub Check: Lint and Test with Node.js 18 on macos-latest
• GitHub Check: Lint and Test with Node.js 18 on windows-latest
• GitHub Check: Lint and Test with Node.js 23 on windows-latest
• GitHub Check: Lint and Test with Node.js 20 on windows-latest
• GitHub Check: Lint and Test with Node.js 22 on windows-latest
• GitHub Check: Lint and Test with Node.js 18 on windows-latest
• GitHub Check: Lint and Test with Node.js 18.18 on windows-latest

test/common.spec.ts (2)

1-77: Well-structured and comprehensive test suite

The test suite is well-structured with good separation of concerns between the hasFlag and compareVersion functionality. Each test case is clearly defined with meaningful descriptions, and the tests cover a variety of scenarios including edge cases.

Good job on implementing proper test isolation with the beforeEach hook that resets the environment before each test.

🧰 Tools

🪛 Biome (1.9.4)

[error] 11-11: Avoid the delete operator which can impact performance.

Unsafe fix: Use an undefined assignment instead.

(lint/performance/noDelete)

---

57-60:

Details

❓ Verification inconclusive

Verify semver comparison behavior for pre-release versions

The test asserts that "1.0.0" and "1.0.0-rc1" are considered equal (returning 0). This might be unexpected if following standard semver comparison rules, which typically treat pre-release versions as less than regular releases.

---

🏁 Script executed:

#!/bin/bash
# Check the implementation of compareVersion in the source code
rg -A 10 "export function compareVersion" --glob "src/*.ts"

Length of output: 61

---

Manual Verification Required: Semver Pre-release Comparison Behavior

The current test in test/common.spec.ts (lines 57–60) checks that compareVersion('1.0.0', '1.0.0-rc1') returns 0, which is non-standard for semver—pre-release versions should typically compare as less than the corresponding release. Our initial attempt to locate the implementation of compareVersion in src/*.ts did not return any output, so it’s unclear if the function is defined elsewhere or implemented with specific custom semantics.

Next Steps:

• Verify the Implementation: Manually check where compareVersion is defined. Ensure that its logic aligns with the intended semver rules. If it is located in a different file or module, please update the search reference accordingly.
• Behavior Confirmation: Confirm whether the current behavior of treating 1.0.0 and 1.0.0-rc1 as equal is intentional. If not, adjust the implementation or tests so that a pre-release (e.g., -rc1) is correctly considered lower than a standard release.

src/constants.ts (5)

31-44: Consider implementing the previously suggested documentation improvements for version constants.

These version constants are critical for feature detection, but lack contextual documentation. As suggested in a previous review, adding JSDoc comments to explain what each version constant corresponds to would significantly improve maintainability and understanding.

---

67-70: Use a more robust boolean parsing approach as previously suggested.

The current implementation checks if the string equals '1' or 'true' but may not handle other truthy values correctly (like 'True', 'TRUE', etc.) as noted in a previous review. Consider implementing the suggested parsing helper function for more consistent behavior.

---

61-61: Apply the previously suggested fix for timeout parsing.

As noted in the previous review and agreed upon, this line should be updated to the suggested implementation for better type safety and readability.

---

4-21: LGTM! Well-organized TypeScript runner definitions.

The TsRunner object is well-structured with clear documentation links for each runner type. The use of 'as const' ensures type safety, and the ValueOf type utility creates a proper union type.

---

71-81: Global shims preset looks good.

The DEFAULT_GLOBAL_SHIMS_PRESET array correctly implements the GlobalShim interface structure, providing reasonable defaults for common use cases like fetch and performance APIs.