fix: syncMode - default: use append strategy when using dev serve…

[xiaweiss]

fix: syncMode - default: use append strategy when using dev server, overwrite strategy when using build.

Description

Setting is: syncMode: "default"

The overwrite was executed when using dev server.

Updated: PR handles the default option within the setupWatcher hook, and dev-server uses append by default.

// setupWatcher, setupViteServer, setupWatcherWebpack
setupWatcher () {
    this._removeUnused = this.options.syncMode === 'overwrite'
    // ...
}

The PR has added an environment variable check to handle the default behavior of "default".

// deprecated
const syncMode = this.options.syncMode === 'default' ? (process.env.NODE_ENV === 'development' ? 'append' : 'overwrite') : this.options.syncMode

Relevant Pull Request

feat: introduce syncMode to control the behavior of generating files

[bolt-new-by-stackblitz]

Run & review this pull request in StackBlitz Codeflow.

[Copilot]

Pull Request Overview

This PR fixes the syncMode: "default" behavior to properly use append strategy during development and overwrite strategy during build. Previously, the default mode was incorrectly using overwrite strategy when using the dev server.

• Introduces environment-based logic to determine the appropriate sync mode when syncMode is set to "default"
• Removes duplicate sync mode assignment that was overriding the intended behavior in dev server setup
• Adds clarifying comment explaining the default behavior logic

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[Copilot]

[nitpick] The ternary operator is nested and difficult to read. Consider extracting this logic into a separate method like resolveSyncMode() for better maintainability.

[xiaweiss]

Okay, I have fixed this issue and moved the logic to the options.ts file.

[kevinmarrec]

@xiaweiss IMO we should never relay on environment variables at this level.

Development server being present does not mean running in development environment, in this Vite Plugin we always relay on a Vite hook that give us the dev server to determine if we're running a dev server.

Just have a look on previous implementation, it was using removeUnused = !this.server

Therefore my opinion is that this changed may even break more use cases.

If there's a bug we should have first a look at a reproduction that show something breaking when upgrading the version.

I don't have more time for now to dive more in this, I'll let @antfu hopefully have a decision about what to do here.

[xiaweiss]

@xiaweiss IMO we should never relay on environment variables at this level.

Development server being present does not mean running in development environment, in this Vite Plugin we always relay on a Vite hook that give us the dev server to determine if we're running a dev server.

Just have a look on previous implementation, it was using removeUnused = !this.server

Therefore my opinion is that this changed may even break more use cases.

If there's a bug we should have first a look at a reproduction that show something breaking when upgrading the version.

I don't have more time for now to dive more in this, I'll let @antfu hopefully have a decision about what to do here.

Thank you very much for your feedback. I will further research and explore how to determine the status of the dev-server.

[xiaweiss]

update: process.env.NODE_ENV has been deprecated, and replaced with the setupWatcher hook.