Skip to content
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.

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

feat: add environment::restart #18353

Closed

Conversation

sapphi-red
Copy link
Member

@sapphi-red sapphi-red commented Oct 15, 2024

Description

Alternative to #18263

close #18264

@sapphi-red sapphi-red added the feat: environment API Vite Environment API label Oct 15, 2024
Copy link

stackblitz bot commented Oct 15, 2024

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

packages/vite/src/node/server/environment.ts Outdated Show resolved Hide resolved
Comment on lines +168 to +169
// TODO: when restart is called, close is not called for the previous environment
// that means depOptimizer is initialized but not closed
Copy link
Member Author

Choose a reason for hiding this comment

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

Would this be a problem? If we move the depOptimizer initialization to init, this won't be a problem. Is that safe to do?

Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should call await this.close() then extract things from the constructor and also call init(). There are some things missing to be re-inited. _crawlEndFinder should be recreated, moduleGraph too. I don't know if we should reuse things. We don't have any long term resource to keep.

@@ -162,6 +166,21 @@ export class DevEnvironment extends BaseEnvironment {
)
}

// TODO: when restart is called, close is not called for the previous environment
// that means depOptimizer is initialized but not closed
async restart(options?: { watcher?: FSWatcher }): Promise<void> {
Copy link
Member Author

Choose a reason for hiding this comment

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

TODO: we should pass the new config here

@sapphi-red
Copy link
Member Author

Closing for now as we're trying #18263 first.

@sapphi-red sapphi-red closed this Oct 17, 2024
@sapphi-red sapphi-red deleted the feat/add-environment-restart branch October 18, 2024 00:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat: environment API Vite Environment API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

A way to instantiate a resource that must be a singleton inside a dev environment
2 participants