chore: Fix last path-to-regexp depenabot alert by using vite preview instead of serve
#11015
Conversation
bnussman-akamai
added
the
Dependencies
bnussman-akamai
requested review from
carrillo-erik and
coliu-akamai
and removed request for
a team
packages/manager/package.json
Outdated
| @@ -85,7 +85,7 @@ | |||
| "scripts": { | |||
| "start": "concurrently --raw \"vite\" \"tsc --watch --preserveWatchOutput\"", | |||
| "start:expose": "concurrently --raw \"vite --host\" \"tsc --watch --preserveWatchOutput\"", | |||
| "start:ci": "yarn serve ./build -p 3000 -s --cors", | |||
| "start:ci": "vite preview --port 3000", | |||
There was a problem hiding this comment.
@jdamore-linode Think this will work?
There was a problem hiding this comment.
If it uses Vite's dev server I suspect it'll cause performance issues in the Cypress tests, but I can double check and confirm.
For context:
cypress-io/cypress#3708
tl;dr, to the best of my understanding: the mechanism Cypress uses to intercept HTTP requests (which applies to all requests that happen during a Cypress run, not just those that are cy.intercept()ed) doesn't support HTTP2. Because of the way Vite's dev server chunks(?) things (an HTTP request for every module), Cloud's performance really suffers in Cypress as a result. It isn't too noticeable when running a single test here and there, but over the course of the full suite it causes a pretty significant impact (and may even cause flakiness in some cases, but might be misremembering).
(This is why I generally suggest yarn && yarn build && yarn start:manager:ci in my test PRs, but I'm going to be formally documenting this as part of the test docs I've been working on [on and off] the past few weeks.)
There was a problem hiding this comment.
(I didn't take the time to look at what this PR is doing, but there's no reason we have to use serve specifically here! It's just the particular combo of Vite's dev server + Cypress that doesn't play nicely)
There was a problem hiding this comment.
I believe vite preview uses a basic Node HTTP server to serve the static build assets. It's not running the Vite's dev server from what I understand.
Let me investigate a little more and confirm that vite preview's performance is going to be comparable to Vercel's serve. I think it should be 🤞
There was a problem hiding this comment.
Ooh, you're right! That's awesome, I'm on board.
(Also see it running in CI -- it's totally on track 👍)
There was a problem hiding this comment.
To be clear, I do like the utility of serve. For example, I run yarn serve packages/manager/storybook-static pretty often to test Storybook builds, but from now on, I'll just do bunx serve packages/manager/storybook-static if I need it
There was a problem hiding this comment.
we could also just run npx/pnpx serve in the pipeline without a local dependency
There was a problem hiding this comment.
discussed async: this is good 👍
| path-to-regexp@2.2.1: | ||
| version "2.2.1" | ||
| resolved "https://registry.yarnpkg.com/path-to-regexp/-/path-to-regexp-2.2.1.tgz#90b617025a16381a879bc82a38d4e8bdeb2bcf45" | ||
| integrity sha512-gu9bD6Ta5bwGrrU8muHzVOBFFREpp2iRkVfhBJahwJ6p6Xw20SjT0MxLnwkjOibQmGSYhiUnf2FLe7k+jcFmGQ== |
There was a problem hiding this comment.
This PR removes our serve devDependency which results in the vulnerable version of path-to-regexp being removed, resolving https://github.com/linode/manager/security/dependabot/111
|
Coverage Report: ✅ |
| component: <Security />, | ||
| }); | ||
| // TODO: Unskip once M3-8559 is addressed. | ||
| it.skip('should render a root password input', async () => { |
There was a problem hiding this comment.
I unskipped these in #11005 but they are still being flakey so I'm going to re-skip them. We have a ticket to further investigate this
bnussman-akamai
changed the title
path-to-regexp depenabot alertpath-to-regexp depenabot alert
bnussman-akamai
changed the title
path-to-regexp depenabot alertpath-to-regexp depenabot alert by using vite preview instead of serve
Cloud Manager E2E
|
||||||||||||||||||||||||||||
| Project |
Cloud Manager E2E
|
| Branch Review |
develop
|
| Run status |
|
| Run duration | 26m 16s |
| Commit |
|
| Committer | Banks Nussman |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
3
|
Pending
|
2
|
Skipped
|
0
|
Passing
|
408
|
| View all changes introduced in this branch ↗︎ | |
Description 📝
serveand uses Vite's built-in http server 🌎Note
We are removing
serveas a dev dependency to satisfy dependabot but if you even need to use it to serve static HTTP assets, you can always run it withnpx/bunxFor example
How to test 🧪
As an Author I have considered 🤔