Skip to content

Move ineffective dynamic import warning from plugin to core#7530

Closed
Copilot wants to merge 7 commits intomainfrom
copilot/move-warning-to-core
Closed

Move ineffective dynamic import warning from plugin to core#7530
Copilot wants to merge 7 commits intomainfrom
copilot/move-warning-to-core

Conversation

Copy link
Contributor

Copilot AI commented Dec 16, 2025

The warning for ineffective dynamic imports was incorrectly placed in the vite_reporter plugin's render_chunk hook. This is core bundling logic that should execute regardless of which reporter is used.

Changes

  • New diagnostic event: Created IneffectiveDynamicImport in rolldown_error with join_with_limit utility for formatting module lists
  • Core detection logic: Added warning check in split_chunks() method during module assignment to chunks, emitting warnings when modules with dynamic importers are placed in the same chunk as those importers
  • Performance optimization: Static method check_ineffective_dynamic_import_for_module() checks each module during chunk assignment without global iteration overhead
  • Plugin cleanup: Removed warning logic from vite_reporter, simplified to pure logging responsibilities

Implementation

The check detects when a module has both static and dynamic importers in the same chunk during the code splitting phase:

// In split_chunks(), when assigning module to existing chunk
if let Some(chunk_id) = bits_to_chunk.get(bits).copied() {
  chunk_graph.add_module_to_chunk(normal_module.idx, chunk_id, ...);
  
  // Check for ineffective dynamic imports at assignment time
  if !self.options.inline_dynamic_imports
    && !normal_module.importers.is_empty()
    && !normal_module.dynamic_importers.is_empty()
  {
    if let Some(warning) = Self::check_ineffective_dynamic_import_for_module(
      normal_module, chunk_id, &chunk_graph, &self.link_output.module_table,
    ) {
      ineffective_warnings.push(warning);
    }
  }
}

The warning is now emitted in code_splitting.rs during the split_chunks() phase (lines 749-804) when modules are actually being assigned to chunks, providing better timing and context. Filters out node_modules to avoid noise from third-party dependencies.

Original prompt

Move this warning to core:

if !args.options.inline_dynamic_imports {
for id in &args.chunk.module_ids {
let Some(module) = ctx.get_module_info(id) else {
continue;
};
// When a dynamic importer shares a chunk with the imported module,
// warn that the dynamic imported module will not be moved to another chunk (#12850).
if !module.importers.is_empty() && !module.dynamic_importers.is_empty() {
// Filter out the intersection of dynamic importers and sibling modules in
// the same chunk. The intersecting dynamic importers' dynamic import is not
// expected to work. Note we're only detecting the direct ineffective dynamic import here.
let detected_ineffective_dynamic_import = module
.dynamic_importers
.iter()
.any(|id| !is_in_node_modules(id.as_path()) && args.chunk.module_ids.contains(id));
if detected_ineffective_dynamic_import {
let dynamic_importers_list: Vec<_> =
module.dynamic_importers.iter().map(std::convert::AsRef::as_ref).collect();
let importers_list: Vec<_> =
module.importers.iter().map(std::convert::AsRef::as_ref).collect();
let message = format!(
"\n(!) {} is dynamically imported by {} but also statically imported by {}, dynamic import will not move module into another chunk.\n",
module.id.as_ref(),
utils::join_with_limit(&dynamic_importers_list, ", ", 5),
utils::join_with_limit(&importers_list, ", ", 5),
);
ctx.warn(rolldown_common::LogWithoutPlugin { message, ..Default::default() });
}
}
}
}


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

@netlify
Copy link

netlify bot commented Dec 16, 2025

Deploy Preview for rolldown-rs canceled.

Name Link
🔨 Latest commit 7fdf753
🔍 Latest deploy log https://app.netlify.com/projects/rolldown-rs/deploys/6942324ab01a05000853f99f

Copilot AI and others added 5 commits December 16, 2025 11:00
- Created new BuildDiagnostic event `IneffectiveDynamicImport`
- Added check_ineffective_dynamic_imports method in GenerateStage
- Removed warning logic from vite_reporter plugin
- Added rolldown_plugin_utils dependency to rolldown crate

Co-authored-by: sapphi-red <49056869+sapphi-red@users.noreply.github.com>
Co-authored-by: sapphi-red <49056869+sapphi-red@users.noreply.github.com>
- Build lookup map of module_id -> chunk modules for O(1) lookups
- Use pointer comparison to check if modules are in same chunk
- Reduced complexity from O(n*m*k) to O(n*m)

Co-authored-by: sapphi-red <49056869+sapphi-red@users.noreply.github.com>
- Store ChunkIdx instead of Vec reference in lookup map
- Use chunk index comparison for more robust checking
- Update comment to accurately reflect what is being stored

Co-authored-by: sapphi-red <49056869+sapphi-red@users.noreply.github.com>
Co-authored-by: sapphi-red <49056869+sapphi-red@users.noreply.github.com>
Copilot AI changed the title [WIP] Move warning implementation to core library Move ineffective dynamic import warning from plugin to core Dec 16, 2025
Copilot AI requested a review from sapphi-red December 16, 2025 11:12
@sapphi-red
Copy link
Member

@copilot can we emit the warning when we actually move the dynamically imported module

Copy link
Contributor Author

Copilot AI commented Dec 17, 2025

@copilot can we emit the warning when we actually move the dynamically imported module

The warning is currently emitted when a module has both static and dynamic importers in the same chunk. This indicates the dynamic import won't split the module into a separate chunk because it's already statically imported.

Could you clarify what you mean by "when we actually move the dynamically imported module"? Are you asking to:

  1. Only warn when the dynamic import would have created a new chunk (i.e., exclude cases where it wouldn't split anyway)?
  2. Move the warning to a different location in the code?
  3. Something else?

The current logic matches the original vite_reporter behavior - it checks if both module.importers and module.dynamic_importers are non-empty and if any dynamic importer is in the same chunk.

@sapphi-red
Copy link
Member

@copilot "2. Move the warning to a different location in the code?" is what I mean

Moved ineffective dynamic import warning from generate() to split_chunks()
where modules are actually assigned to chunks. This provides better context
and timing for the warning - it's now emitted when we assign a module to
a chunk and detect it has dynamic importers in the same chunk.

Changes:
- Removed check from GenerateStage::generate()
- Added warning check in split_chunks() loop during module assignment
- Made check_ineffective_dynamic_import_for_module a static method
- Collect warnings during loop and add to link_output after
- Changed split_chunks to &mut self to allow warning collection

Co-authored-by: sapphi-red <49056869+sapphi-red@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Dec 17, 2025

@copilot "2. Move the warning to a different location in the code?" is what I mean

Moved the warning emission to the code splitting phase in commit 7fdf753.

The warning is now emitted in split_chunks() (lines 749-804 in code_splitting.rs) when modules are actually being assigned to chunks. This provides better timing and context - the warning fires exactly when we discover that a module with dynamic importers is being placed in the same chunk as those importers.

The key change is checking for ineffective dynamic imports right after adding a module to an existing chunk (line 777-789), where we can immediately detect if any of its dynamic importers are also in that chunk.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants