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: new hook configurePreviewServer #7658

Merged
merged 2 commits into from May 7, 2022

Conversation

brillout
Copy link
Contributor

@brillout brillout commented Apr 9, 2022

Description

Same than configureServer but for the preview server.

Additional context

We are building frameworks on top of vps and we are aiming for zero-config developer experiences. For example:

// package.json
{
  "scripts": {
    "dev": "vite",
    "build": "vite build",
    "preview": "vite preview"
  },
  "dependencies": {
    "vite": "^2.9.1",
    "stem": "0.0.1"
  },
}

To make this work we need to add the vps and Telefunc middlewares to Vite's dev server which we can achieve by using configureServer(). The $ vite build also works. The only command that doesn't work is $ vite preview because there are currently no way to inject middlewares to the preview server.


What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Read the Pull Request Guidelines and follow the Commit Convention.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Ideally, include relevant tests that fail without this PR but pass with it.

I considered adding tests, but the only meaningful tests I could think of were e2e tests. I think a new e2e test would be too much; it would increase the number of playgrounds without adding enough value to be justified. The vps test suite will include a test, so we'll catch problems at latest when running Vite against vps's test suite.

@brillout brillout force-pushed the feat/configurePreviewServer branch from 3521d97 to 64e560e Compare April 9, 2022 22:11
docs/guide/api-plugin.md Outdated Show resolved Hide resolved
@patak-dev patak-dev added the p3-downstream-blocker Blocking the downstream ecosystem to work properly (priority) label Apr 10, 2022
patak-dev
patak-dev previously approved these changes Apr 10, 2022
@brillout brillout mentioned this pull request Apr 11, 2022
9 tasks
@ch99q
Copy link

ch99q commented Apr 11, 2022

This would make an SSR plugin doable without making a wrapper CLI for Vite! Awesome feature! 🔥🚀

@antfu
Copy link
Member

antfu commented Apr 11, 2022

Instead of a new hook, I am thinking we could either:

{
  name: 'myplugin',
  configureServer(server, mode) {
    if (mode === 'preview') {
      //...
    }
  }
}

or

{
  name: 'myplugin',
  apply: 'preview', // normal plugins won't apply to preview unless set explictly
  configureServer(server) {
     //...
  }
}

@brillout
Copy link
Contributor Author

Instead of a new hook

I share the sentiment and it was actually my initial implementation. But I'm not sure how we could make it work with TypeScript:

type PreviewServer = { middlewares: Connect.Server, httpServer: http.Server }
{
  name: 'myplugin',
  // Is `server` a `ViteDevServer` or a `PreviewServer`?
  configureServer(server) {
      //...
  }
}

Setting server to ViteDevServer | PreviewServer is not ideal for DX. I'd argue that TypeScript DX justifies the creation of a new hook.

This would make an SSR plugin doable without making a wrapper CLI for Vite! Awesome feature! 🔥🚀

Exactly. This is actually what is happening with vps: https://github.com/brillout/vite-plugin-ssr/blob/0.4.0/examples/react/package.json — note how the vite CLI is used together with vite-plugin-ssr. (The prod script is a misnomer used for vps's CI.). You may also want to watch this PR #7665.

@brillout
Copy link
Contributor Author

Rebased & added some code comments.

@bluwy
Copy link
Member

bluwy commented Apr 25, 2022

Note: This PR is in the team discussion board. We'll try to get to it soon!

@Niputi Niputi linked an issue Apr 28, 2022 that may be closed by this pull request
@patak-dev
Copy link
Member

@brillout we discussed today this proposal, we think it is a good idea. Would you rebase the PR? Then we are good to merge it.

brillout and others added 2 commits May 7, 2022 09:00
Co-authored-by: Jeff Yang <32727188+ydcjeff@users.noreply.github.com>
@brillout brillout force-pushed the feat/configurePreviewServer branch from 5d55913 to 38c7701 Compare May 7, 2022 07:01
@brillout
Copy link
Contributor Author

brillout commented May 7, 2022

Done.

Btw. I'm not forgetting to resurrect the $ vite preview SSR fix, I'll just do it after the vite-plugin-ssr 0.4 release.

Note: This PR is in the team discussion board. We'll try to get to it soon!

Thanks for the update, that was super helpful :-).

@brillout
Copy link
Contributor Author

brillout commented May 7, 2022

Btw. I'm not forgetting to resurrect the $ vite preview SSR fix, I'll just do it after the vite-plugin-ssr 0.4 release.

I couldn't contain myself: #8061 :-).

@patak-dev patak-dev merged commit 20ea999 into vitejs:main May 7, 2022
benmccann pushed a commit to benmccann/vite that referenced this pull request Jun 1, 2022
patak-dev pushed a commit that referenced this pull request Jun 2, 2022
Co-authored-by: Rom <git@brillout.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p3-downstream-blocker Blocking the downstream ecosystem to work properly (priority)
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Support custom middleware for preview server
6 participants