Skip to content

fix(parser): do not skip parsing expression inside require.resolve#11133

Merged
JSerFeng merged 1 commit intomainfrom
requireresolveskip
Jul 22, 2025
Merged

fix(parser): do not skip parsing expression inside require.resolve#11133
JSerFeng merged 1 commit intomainfrom
requireresolveskip

Conversation

@fi3ework
Copy link
Member

Summary

when i was migrating Rsdoctor from Modern Module to Rslib, i found the following code and its wrong output due to the skip of expression in require.resolve

source:

import path from 'path';

export function loadLoaderModule(loaderPath, cwd = process.cwd()) {
  const mod = require(process.env.A
    ? path.resolve(cwd, cleanLoaderPath)
    : require.resolve(cleanLoaderPath, {
        paths: [cwd, path.resolve(cwd, 'node_modules')],
      }));

  return 1;
}

output:

// ...
const external_path_namespaceObject = require("path");
var external_path_default = /*#__PURE__*/ __webpack_require__.n(external_path_namespaceObject);
function loadLoaderModule(loaderPath, cwd = process.cwd()) {
    require(process.env.DOCTOR_TEST ? external_path_default().resolve(cwd, cleanLoaderPath) : require.resolve(cleanLoaderPath, {
        paths: [
            cwd,
            path.resolve(cwd, 'node_modules') // 🔴 path is bind to external_path_namespaceObject
        ]
    }));
    return external_path_default().resolve('q');
}
// ...

Related links

Checklist

  • Tests updated (or not required).
  • Documentation updated (or not required).

Copilot AI review requested due to automatic review settings July 22, 2025 12:10
@netlify
Copy link

netlify bot commented Jul 22, 2025

Deploy Preview for rspack canceled.

Name Link
🔨 Latest commit 2279fde
🔍 Latest deploy log https://app.netlify.com/projects/rspack/deploys/687f80e68677b000084f8c0d

@github-actions github-actions bot added team The issue/pr is created by the member of Rspack. release: bug fix release: bug related release(mr only) labels Jul 22, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a parser bug where expressions inside require.resolve() calls were being skipped when requireResolve parsing was disabled. The fix ensures that expressions within require.resolve() are still parsed and transformed properly even when the require.resolve feature itself is disabled.

Key changes:

  • Moved the require.resolve configuration check to occur after expression parsing
  • Added test cases to verify expressions inside require.resolve() are properly transformed
  • Updated existing test expectations to include the new transformation behavior

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
crates/rspack_plugin_javascript/src/parser_plugin/common_js_imports_parse_plugin.rs Moved the require_resolve configuration check from process_resolve method to the plugin hook to allow expression parsing
packages/rspack-test-tools/tests/configCases/parsing/require-resolve/index.js Added test cases with path.resolve() expressions inside require.resolve() calls
packages/rspack-test-tools/tests/configCases/parsing/require-resolve/test.js Added test expectations for the new transformation behavior

@fi3ework fi3ework force-pushed the requireresolveskip branch from c2d8154 to 2279fde Compare July 22, 2025 12:15
@github-actions
Copy link
Contributor

📦 Binary Size-limit

Comparing 2279fde to chore: degrade napi-build to 2.2.2 (#11130) by CPunisher

🙈 Size remains the same at 48.97MB

@fi3ework fi3ework requested a review from JSerFeng July 22, 2025 12:34
@codspeed-hq
Copy link

codspeed-hq bot commented Jul 22, 2025

CodSpeed Performance Report

Merging #11133 will not alter performance

Comparing requireresolveskip (2279fde) with main (e1f8f4d)

Summary

✅ 16 untouched benchmarks

@JSerFeng JSerFeng merged commit 7f849d6 into main Jul 22, 2025
44 checks passed
@JSerFeng JSerFeng deleted the requireresolveskip branch July 22, 2025 18:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release: bug fix release: bug related release(mr only) team The issue/pr is created by the member of Rspack.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants