Skip to content

fix(rspack): remove virtual plugin cleanup signal handlers - #541

Merged
sxzz merged 2 commits into
unjs:mainfrom
elbywan:fix/rspack-remove-signal-handlers
Aug 27, 2025
Merged

fix(rspack): remove virtual plugin cleanup signal handlers#541
sxzz merged 2 commits into
unjs:mainfrom
elbywan:fix/rspack-remove-signal-handlers

Conversation

@elbywan

@elbywan elbywan commented Aug 27, 2025

Copy link
Copy Markdown
Contributor

This PR appears to have broken other apps TanStack/router#5023

Removing the cleanup handlers added in #538.

@pkg-pr-new

pkg-pr-new Bot commented Aug 27, 2025

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/unjs/unplugin@541

commit: 768f3e2

Comment thread src/rspack/utils.ts Outdated
this.counters.forEach((_, dir) => {
fs.rmSync(dir, { recursive: true, force: true })
})
process.once('exit', () => {

@sxzz sxzz Aug 27, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we move this part to the constructor (or other lazy approach)? The static block will always be executed, regardless of whether rspack is used.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sure, is something like this okay?

Comment thread src/rspack/utils.ts Outdated
})
})
}
process.once('exit', FakeVirtualModulesPlugin.cleanupFn)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Make sure this call will also be execute once.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

if the flag is true, skip it. Instead of store a function.

Comment thread src/rspack/utils.ts Outdated
export class FakeVirtualModulesPlugin {
name = 'FakeVirtualModulesPlugin'
static counters: Map<string, number> = new Map<string, number>()
static cleanupFn: (() => void) | null = null

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

let this flag be a boolean

@elbywan elbywan Aug 27, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

squashed here: 768f3e2

@elbywan
elbywan force-pushed the fix/rspack-remove-signal-handlers branch from c48bb90 to 00db236 Compare August 27, 2025 07:53
@elbywan
elbywan force-pushed the fix/rspack-remove-signal-handlers branch from 00db236 to 768f3e2 Compare August 27, 2025 08:04
@sxzz
sxzz merged commit ca42080 into unjs:main Aug 27, 2025
15 checks passed
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