Skip to content

fix: circular barrel file#115

Closed
fireairforce wants to merge 4 commits intoutoofrom
fix-circular-barrel-file
Closed

fix: circular barrel file#115
fireairforce wants to merge 4 commits intoutoofrom
fix-circular-barrel-file

Conversation

@fireairforce
Copy link
Member

No description provided.

@gemini-code-assist
Copy link

Summary of Changes

Hello @fireairforce, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request refines the handling of re-export statements within the Turbopack ECMAScript module analysis to prevent circular dependencies, particularly those arising from 'barrel files'. It introduces a distinct mechanism to identify and process module evaluations that originate from re-exports, ensuring they are not eagerly imported by local references. This change improves the accuracy of dependency analysis and enhances the robustness of module resolution by avoiding unintended dependency cycles.

Highlights

  • Re-export Evaluation Flag: Introduced a new boolean field, is_reexport_evaluation, to the ImportMapReference struct to explicitly mark import references that originate from re-export statements.
  • Dedicated Re-export Reference Creation: Implemented a new private function, ensure_reexport_evaluation_reference, within the Analyzer to create and manage ImportMapReference instances specifically for re-exported modules, ensuring they are correctly flagged.
  • Analyzer Integration: Updated the Visit for Analyzer implementation to utilize the new ensure_reexport_evaluation_reference function when processing export ... from ... statements, ensuring re-exports are handled distinctly.
  • Builder Method for Re-exports: Added a new public method, add_esm_reexport_evaluation_reference, to AnalyzeEcmascriptModuleResultBuilder to allow for the specific addition of re-export evaluation references to the analysis result.
  • Conditional Evaluation Reference Handling: Modified the analyze_ecmascript_module_internal function to conditionally use the new add_esm_reexport_evaluation_reference method based on the is_reexport_evaluation flag. This prevents re-exported modules from being eagerly imported by local references, which could otherwise lead to circular dependencies.
  • Exported Type: Exported the FoundExportType enum from the esm::export module, making it publicly accessible.
Changelog
  • turbopack/crates/turbopack-ecmascript/src/analyzer/imports.rs
    • Added is_reexport_evaluation: bool field to ImportMapReference struct.
    • Implemented ensure_reexport_evaluation_reference function to create import map references for re-exports.
    • Modified Visit for Analyzer to call ensure_reexport_evaluation_reference for export ... from ... statements.
  • turbopack/crates/turbopack-ecmascript/src/references/mod.rs
    • Exported FoundExportType from esm::export module.
    • Added add_esm_reexport_evaluation_reference method to AnalyzeEcmascriptModuleResultBuilder.
    • Updated analyze_ecmascript_module_internal to use is_reexport_evaluation flag for conditional reference addition.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request addresses an issue with circular dependencies in barrel files by introducing a way to distinguish re-export evaluations. A new is_reexport_evaluation flag is added to ImportMapReference, which is then used to route these evaluations to a separate logic path, avoiding the eager imports that lead to dependency cycles. The implementation appears sound and directly targets the problem. I've included one suggestion to improve code clarity and reduce duplication in one of the modified files.

Comment on lines +465 to +471
if let Some(i) = self.data.references.get_index_of(&r) {
Some(i)
} else {
let i = self.data.references.len();
self.data.references.insert(r);
Some(i)
}

Choose a reason for hiding this comment

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

medium

This logic can be simplified by using IndexSet::insert_full, which returns the index of the item whether it was newly inserted or already existed. This avoids the manual check and insertion logic.

The same simplification can be applied to the new ensure_reexport_evaluation_reference function.

        let (i, _) = self.data.references.insert_full(r);
        Some(i)

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