Skip to content

refactor!: serve command renamed to dev#5097

Closed
patak-cat wants to merge 1 commit into
mainfrom
serve-renamed-dev
Closed

refactor!: serve command renamed to dev#5097
patak-cat wants to merge 1 commit into
mainfrom
serve-renamed-dev

Conversation

@patak-cat

Copy link
Copy Markdown
Member

Description

In the last team meeting we discussed renaming the vite serve command to vite dev.
We currently have

vite / vite serve   / pnpm run dev   | the dev server
       vite build   / pnpm run build | the build command
       vite preview / pnpm run serve | the preview server

It is confusing that pnpm run serve doesn't call vite serve but vite preview.

I think that dev looks good here, but this PR is marked as a draft because I don't know if we can proceed with the renaming. For this change to make sense, we should not only rename the command but also change 'serve' to 'dev' in the plugin and javascript API of Vite. If not, it is going to be equally confusing. The problem is that I don't see a way to make this change backward compatible. Plugins are using apply: 'serve' and are conditionally changing the config using command === 'serve'. The ecosystem may still be small enough to go through with this (by coordinating with plugin authors), but I don't think this is really workable.

I sent the PR anyways because it is easier to discuss by looking at the changeset.

In my opinion, we should leave serve as is. Rename pnpm run dev to pnm run serve and look for a different word for pnpm run {preview}. We can't use preview because of the pre/post scripts. Maybe show, stage? It isn't easy to find a better word than preview. If we find another word we could rename also the command so we end up with

vite / vite serve  / pnpm run serve | the dev server
       vite build  / pnpm run build | the build command
       vite stage  / pnpm run stage | the preview server

What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

@patak-cat patak-cat marked this pull request as draft September 26, 2021 19:19
@Shinigami92

Copy link
Copy Markdown
Member

I personally don't like pnpm run serve to serve preview, cause this would indicate a bit that it is somewhat safe to serve this in production.

stage would be a choice... what about demo? I would really prefer to tell the developer that this is really not for production.

@haoqunjiang

Copy link
Copy Markdown
Member

stage and demo both look good to me.

But I prefer demo, in case users having a .env.staging file and a build:staging command. demo is less likely to cause confusion in that case.

@patak-cat

patak-cat commented Oct 2, 2021

Copy link
Copy Markdown
Member Author

I see that Astro is using preview as the script name for astro preview. SvelteKit is also using preview for this. @dominikg did you run into any issues with this choice?
@sodatea what was the problematic case you described with it? Shouldn't this only bring issues in case users would use view as a script name?

As an aside, both Astro and SvelteKit use dev instead of serve.

@dominikg

dominikg commented Oct 3, 2021

Copy link
Copy Markdown
Contributor

I like consistency and dev/build/preview for both script and command names makes sense.

renaming vite serve to dev (both command and mode) would achieve this. keeping a serve alias around with a deprecation warning until vite3

@dominikg

dominikg commented Oct 3, 2021

Copy link
Copy Markdown
Contributor

Also I completely agree that the preview command should be explicitly documented as "do not use in production" and it transpires that message in the name itself. serve could be misunderstood as ok for production use

@haoqunjiang

Copy link
Copy Markdown
Member

Shouldn't this only bring issues in case users would use view as a script name?

Both view and prepreview behave inconsistently in different package managers.


The case for prepreview is that I need build to be run before preview in E2E testing.
But now that pnpm stops supporting pre-scripts by default pnpm/pnpm#2891, I think I'll use npm-run-all instead.


So for most users, yes, I think we only need to remind them of avoiding naming scripts as view

@patak-cat

Copy link
Copy Markdown
Member Author

Closing in favor of #5207

@patak-cat patak-cat closed this Oct 10, 2021
@antfu antfu deleted the serve-renamed-dev branch December 24, 2021 05:04
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.

4 participants