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

vite: Create compiler during buildStart instead of configResolved #1353

Merged
merged 4 commits into from
Mar 8, 2024

Conversation

himself65
Copy link
Contributor

@himself65 himself65 commented Mar 8, 2024

Related: dai-shi/waku#559

Copy link

changeset-bot bot commented Mar 8, 2024

🦋 Changeset detected

Latest commit: d9d4f9f

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@vanilla-extract/vite-plugin Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@himself65 himself65 changed the title fix: compiler might be hangup fix: compiler cannot cleanup Mar 8, 2024
@askoufis
Copy link
Contributor

askoufis commented Mar 8, 2024

@himself65 Thanks for the PR. Could you provide some more context on what the issue is and how this change fixes it?

I can sort of see how createCompiler is better placed in the buildStart() hook, but I don't think the !compiler check is achieving anything here. Were you observing the compiler being recreated, leading to a bug?

@himself65
Copy link
Contributor Author

himself65 commented Mar 8, 2024

@himself65 Thanks for the PR. Could you provide some more context on what the issue is and how this change fixes it?

I can sort of see how createCompiler is better placed in the buildStart() hook, but I don't think the !compiler check is achieving anything here. Were you observing the compiler being recreated, leading to a bug?

Yeah, the issue is when we call resolveConfig in node.js, buildEnd won't be called in this case, so that the node.js process cannot exit successfully

@himself65
Copy link
Contributor Author

!compiler check

Oh, that is my mistake

@askoufis
Copy link
Contributor

askoufis commented Mar 8, 2024

@himself65 Thanks for the PR. Could you provide some more context on what the issue is and how this change fixes it?
I can sort of see how createCompiler is better placed in the buildStart() hook, but I don't think the !compiler check is achieving anything here. Were you observing the compiler being recreated, leading to a bug?

Yeah, the issue is when we call resolveConfig in node.js, buildEnd won't be called in this case, so that the node.js process cannot exit successfully

Oh I understand now. So resolveConfig calls the configResolved hooks of plugins, so a compiler is created but never destroyed. Makes sense to me. I'll add a changeset.

@askoufis askoufis changed the title fix: compiler cannot cleanup vite: Create compiler during buildStart instead of configResolved Mar 8, 2024
@askoufis askoufis enabled auto-merge (squash) March 8, 2024 04:51
@askoufis askoufis merged commit 94d5f06 into vanilla-extract-css:master Mar 8, 2024
5 checks passed
@askoufis
Copy link
Contributor

askoufis commented Mar 8, 2024

@himself65 himself65 deleted the fix-config branch March 21, 2024 21:38
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