Skip to content

fix: pass null to additional data of loader context#10155

Merged
LingyuCoder merged 2 commits intomainfrom
fix/val-loader-error
Apr 24, 2025
Merged

fix: pass null to additional data of loader context#10155
LingyuCoder merged 2 commits intomainfrom
fix/val-loader-error

Conversation

@LingyuCoder
Copy link
Contributor

Summary

fix #9635

same as https://github.com/web-infra-dev/rspack/blob/main/packages/rspack/src/loader-runner/index.ts#L1023

Checklist

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

@LingyuCoder LingyuCoder requested review from Copilot and h-a-n-a April 24, 2025 07:05
@github-actions github-actions bot added release: bug fix release: bug related release(mr only) team The issue/pr is created by the member of Rspack. labels Apr 24, 2025
@netlify
Copy link

netlify bot commented Apr 24, 2025

Deploy Preview for rspack canceled.

Name Link
🔨 Latest commit d338bf1
🔍 Latest deploy log https://app.netlify.com/sites/rspack/deploys/6809e29e20bf710008361996

@LingyuCoder LingyuCoder changed the title fix: should not fail when pass null to additional data of async loader context fix: should not fail when pass null to additional data of loader context Apr 24, 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 pull request addresses an issue where passing a null value to the additionalData parameter of the async loader context causes the process to fail. It fixes the assignment in the loader runner and adds a new test case to verify that null values are handled properly.

  • Changed the assignment of context.additionalData in loader-runner to safely handle null values.
  • Added test cases and configuration updates in the rspack-test-tools package to confirm the fix.

Reviewed Changes

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

Show a summary per file
File Description
packages/rspack/src/loader-runner/index.ts Modified additionalData assignment to prevent failures when null is passed
packages/rspack-test-tools/tests/configCases/loader/additional-data-async/rspack.config.js Added configuration for async loader additional data test
packages/rspack-test-tools/tests/configCases/loader/additional-data-async/loader.js Loader implementation for testing the additionalData behavior
packages/rspack-test-tools/tests/configCases/loader/additional-data-async/index.js Test case to validate handling of null as additionalData
packages/rspack-test-tools/tests/configCases/loader/additional-data-async/a.js Test input file

@LingyuCoder LingyuCoder changed the title fix: should not fail when pass null to additional data of loader context fix: pass null to additional data of loader context Apr 24, 2025
@netlify
Copy link

netlify bot commented Apr 24, 2025

Deploy Preview for rspack canceled.

Name Link
🔨 Latest commit 2f41c36
🔍 Latest deploy log https://app.netlify.com/sites/rspack/deploys/6809e2a25cf49f0008c4f4da

@h-a-n-a
Copy link
Contributor

h-a-n-a commented Apr 24, 2025

I don't quite get this issue. Would you please point me out the exact case we're after? I can't grab much information from the test case in this PR.

@codspeed-hq
Copy link

codspeed-hq bot commented Apr 24, 2025

CodSpeed Performance Report

Merging #10155 will not alter performance

Comparing fix/val-loader-error (2f41c36) with main (dbdfeb1)

Summary

✅ 11 untouched benchmarks

@LingyuCoder
Copy link
Contributor Author

LingyuCoder commented Apr 24, 2025

I don't quite get this issue. Would you please point me out the exact case we're after? I can't grab much information from the test case in this PR.

@h-a-n-a The val-loader will pass null to loaderContext.additionalData and null can not be transform from node binding (only accept undefined as empty value) which leads to the error of that issue.

@LingyuCoder LingyuCoder enabled auto-merge (squash) April 24, 2025 07:27
@LingyuCoder LingyuCoder merged commit 3688244 into main Apr 24, 2025
34 checks passed
@LingyuCoder LingyuCoder deleted the fix/val-loader-error branch April 24, 2025 07:36
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.

[Bug]: val-loader and val-loader-modernizr fails

3 participants