Skip to content

feat(hmr): check boundary outside the circular imports#21624

Open
sapphi-red wants to merge 2 commits intomainfrom
feat/hmr-check-boundary-outside-the-circular-imports
Open

feat(hmr): check boundary outside the circular imports#21624
sapphi-red wants to merge 2 commits intomainfrom
feat/hmr-check-boundary-outside-the-circular-imports

Conversation

@sapphi-red
Copy link
Member

When a HMR boundary is detected within a circular import chain, skip it and continue propagation outward to find a boundary outside the chain.

graph LR
    B["b.js"] -->|import| C["c.js (boundary)"]
    C -->|import| B
    A["a.js (boundary)"] -->|import| B
Loading

When c.js is changed, c.js is now ignored and a.js is now treated as the boundary.

fixes #16580
fixes #18217

@sapphi-red sapphi-red added feat: hmr p2-nice-to-have Not breaking anything but nice to have (priority) labels Feb 12, 2026
@sapphi-red sapphi-red marked this pull request as draft February 12, 2026 10:21
Comment on lines +697 to +706
} else if (traversedModules.size > 10) {
environment.logger.warn(
colors.yellow(
`hmr boundary skipped due to circular imports, ` +
`${traversedModules.size} modules will re-execute. ` +
`To debug and break the circular import, use ${colors.dim('`vite --debug hmr`')} to log the circular import path.`,
),
{ timestamp: true },
)
}
Copy link
Member

Choose a reason for hiding this comment

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

Can you explain this part and why this PR is removing the warning on the client-side? isWithinCircularImports was introduced for #15118 so we reload the page in case the circular modules cannot recover in the client, does that not happen in this PR anymore?

Copy link
Member Author

Choose a reason for hiding this comment

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

It doesn't happen anymore. If there's a circular import, this PR will make Vite to pick a boundary outside of the circular import. If there's no boundary outside the circular import, it'll trigger a reload. So there won't be a client side error caused by circular imports.

Copy link
Member

Choose a reason for hiding this comment

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

I see. So a minor difference I think is that as you mentioned, if:

  1. There's a boundary in a circular import
  2. There's no boundary outside of the circular import

It will now always reload the page. Previously it would try doing an inner boundary HMR first, before falling back to reload.

So, I don't know how Linear's import structure works, but if they have something like index.html -> main.ts -> App.tsx -> ... -> App.tsx, then I assume this will now always trigger a reload, which isn't something they want to happen.


I'm happy to try the force reload instead so that the execution order is always correct, but if we have a request for the previous behavior again, maybe we need to bring the handling back.

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

Labels

feat: hmr p2-nice-to-have Not breaking anything but nice to have (priority)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

HMR does not update the content when using react and there's a circular import Circular imports in HMR

2 participants