Skip to content

fix: fix panic caused by missing lazy dependency #12820

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
hardfist merged 1 commit into from
Jan 23, 2026
Merged

fix: fix panic caused by missing lazy dependency #12820

hardfist merged 1 commit into from
Jan 23, 2026

Conversation

@hardfist
Copy link
Contributor

@hardfist hardfist commented Jan 23, 2026 •
edited
Loading

Summary

revert this change https://github.com/web-infra-dev/rspack/pull/12569/changes#diff-8543abddbc5dd4ac766da9c7bddf6cd7abc79c702ad8999576cf11e7d663341cL40
but we still needs to figure out why lazy dependency is missing here

Related links

Checklist

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

Copilot AI review requested due to automatic review settings January 23, 2026 03:50
@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 Jan 23, 2026
@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Jan 23, 2026

Deploying rspack-v2 with  Cloudflare Pages  Cloudflare Pages

Latest commit: e939855
Status: ✅  Deploy successful!
Preview URL: https://2a399d9f.rspack-v2.pages.dev
Branch Preview URL: https://yj-fix-lazy-dependency.rspack-v2.pages.dev

View logs

Copilot AI reviewed Jan 23, 2026
View reviewed changes
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 addresses a panic when processing "lazy" dependencies in the module graph by making access to dependencies fallible in the lazy repair path, avoiding crashes when a dependency is unexpectedly missing.

Changes:

  • Add internal::try_dependency_by_id_mut to provide an Option<&mut BoxDependency> access path into ModuleGraph dependencies.
  • Update ProcessUnlazyDependenciesTask to use try_dependency_by_id_mut and skip missing dependencies instead of panicking, preventing crashes when lazy dependencies cannot be found.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
crates/rspack_core/src/module_graph/internal.rs Introduces a fallible mutable dependency accessor (try_dependency_by_id_mut) alongside existing internal helpers for controlled, non-panicking access to ModuleGraph dependencies.
crates/rspack_core/src/compilation/build_module_graph/graph_updater/repair/lazy.rs Switches the un-lazy processing logic to use the new fallible accessor and skip missing dependencies, eliminating the panic during lazy dependency repair.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

chatgpt-codex-connector[bot]
chatgpt-codex-connector bot reviewed Jan 23, 2026
View reviewed changes
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e9398551f7

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@github-actions
Copy link
Contributor

github-actions bot commented Jan 23, 2026

📦 Binary Size-limit

Comparing e939855 to refactor: remove misleadning naming and adjust cache call place (#12818) by hardfist

❌ Size increased by 325.50KB from 47.69MB to 48.01MB (⬆️0.67%)

@codspeed-hq
Copy link

codspeed-hq bot commented Jan 23, 2026

CodSpeed Performance Report

Merging this PR will not alter performance

Comparing yj/fix-lazy-dependency (e939855) with main (30d5897)1

Summary

✅ 16 untouched benchmarks
⏩ 1 skipped benchmark2

Footnotes

  1. No successful run was found on v1.x (29e0a7f) during the generation of this report, so main (30d5897) was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

  2. 1 benchmark was skipped, so the baseline result was used instead. If it was deleted from the codebase, click here and archive it to remove it from the performance reports. ↩

@hardfist hardfist merged commit ebdfb97 into v1.x Jan 23, 2026
58 of 61 checks passed
@hardfist hardfist deleted the yj/fix-lazy-dependency branch January 23, 2026 06:02
@colinaaa colinaaa mentioned this pull request Jan 26, 2026
Merged
3 tasks
colinaaa added a commit to lynx-family/lynx-stack that referenced this pull request Jan 26, 2026
@colinaaa
chore: disable build cache for bundleless (#2139)
99c412f 
Rspack would panic under bundleless mode with persistent cache enabled.

```
pnpm --filter @lynx-js/gesture-runtime run build

> @lynx-js/gesture-runtime@2.1.2 build /Users/colin/lynx-stack/packages/lynx/gesture-runtime
> rslib build

Rslib v0.19.1

info    build started...
Panic occurred at runtime. Please file an issue on GitHub with the backtrace below: https://github.com/web-infra-dev/rspack/issues: panicked at crates/rspack_core/src/module_graph/mod.rs:582:26:
Dependency with ID DependencyId(125) not found
/Users/colin/lynx-stack/packages/lynx/gesture-runtime:
 ERR_PNPM_RECURSIVE_RUN_FIRST_FAIL  @lynx-js/gesture-runtime@2.1.2 build: `rslib build`
Command failed with signal "SIGABRT"
start   generating declaration files... (esm)
fish: Job 1, 'pnpm --filter @lynx-js/gesture-…' terminated by signal SIGABRT (Abort)
```

This should be fixed by
web-infra-dev/rspack#12820.

<!--
  Thank you for submitting a pull request!

We appreciate the time and effort you have invested in making these
changes. Please ensure that you provide enough information to allow
others to review your pull request.

Upon submission, your pull request will be automatically assigned with
reviewers.

If you want to learn more about contributing to this project, please
visit:
https://github.com/lynx-family/lynx-stack/blob/main/CONTRIBUTING.md.
-->

<!-- The AI summary below will be auto-generated - feel free to replace
it with your own. -->

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Chores**
* Modified build performance configuration settings to adjust caching
behavior during the build process.

<sub>✏️ Tip: You can customize this high-level summary in your review
settings.</sub>
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

## Checklist

<!--- Check and mark with an "x" -->

- [ ] Tests updated (or not required).
- [ ] Documentation updated (or not required).
- [ ] Changeset added, and when a BREAKING CHANGE occurs, it needs to be
clearly marked (or not required).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@chatgpt-codex-connector chatgpt-codex-connector[bot] chatgpt-codex-connector[bot] left review comments

@stormslowly stormslowly stormslowly approved these changes

Assignees

No one assigned

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@hardfist @stormslowly