fix: changed the Vite port to the new Vite default port 5173 #792

nickytonline · 2022-07-22T20:15:37Z

🎉 Thanks for submitting a pull request! 🎉

Summary

Fixes #791

Vite changed the default port for from 3000 to 5173 on May 31st or around that time. See

https://github.com/vitejs/vite/blob/2826303bd253e20df2746f84f6a7c06cb5cf3d6b/docs/guide/migration.md#dev-server-changes

vitejs/vite#8417

For us to review and ship your PR efficiently, please perform the following steps:

Open a bug/issue before writing your code 🧑‍💻. This
ensures we can discuss the changes and get feedback from everyone that should be involved. If you`re fixing a
typo or something that`s on fire 🔥 (e.g. incident related), you can skip this step.
Read the contribution guidelines 📖. This ensures your code follows our style guide and
passes our tests.
Update or add tests (if any source code was changed or added) 🧪
Update or add documentation (if features were changed or added) 📝
Make sure the status checks below are successful ✅

A picture of a cute animal (not mandatory, but encouraged)

ascorbic · 2022-07-22T20:32:29Z

I don't think we have a way of handling different versions, do we? Presumably this will cause the reverse problem for older versions of Vite. I guess that's less of a problem because new projects are more likely to use new Vite, but it's worth noting.

nickytonline · 2022-07-22T21:00:17Z

I don't think we have a way of handling different versions, do we? Presumably this will cause the reverse problem for older versions of Vite. I guess that's less of a problem because new projects are more likely to use new Vite, but it's worth noting.

@ascorbic, yeah it's a good point. Have we handled something like this in the past? I'm not sure how problematic it will be. We can definitely make support aware of the issue.

Maybe a notice in the CLI of the port upgrade via a new field in the framework info JSON file if it fails via timeout to find the server?

{
  "id": "vite",
  "name": "Vite",
  "category": "build_tool",
  ...
  "dev": {
    "command": "vite",
    "port": 5173,
    "pollingStrategies": [{ "name": "TCP" }]
	"migrationMessage": "Vite's default port is now 5173. See the migration guide https://vitejs.dev/guide/migration.html#architecture-changes-and-legacy-options"
  },
  ...
}

We could also check for both ports, but that seems more of bandaid solution.

ascorbic · 2022-07-22T21:43:19Z

There is an env var field available, so maybe we can use one to force the old version to use the new port number (or vice versa). Worth a try.

ericapisani

Nice find, LGTM 🚀

nickytonline · 2022-07-25T12:31:08Z

src/frameworks/qwik.json

@@ -9,7 +9,7 @@
  },
  "dev": {
    "command": "vite",
-    "port": 3000,
+    "port": 5173,


For this one @ascorbic, it looks like Qwik has set the port to 5173 if you create a new Qwik app. Since this project is so new, I imagine for this config change it's OK?

nickytonline · 2022-07-25T12:37:00Z

src/frameworks/vite.json

@@ -9,7 +9,7 @@
  },
  "dev": {
    "command": "vite",


@ascorbic, I thought about this one some more, and since this is the default if they have nothing configured, we can explicitly set it to "command": "vite --port 5173",? That way anyone who was using it previously won't run into any issues and new folks won't run into issues either.

If we were to use an environment variable, we'd have to use a vite config to consume it to set the port. The only problem with this is there's a disconnect between the command and the port fields, although this is a minor issue. We could also remove the --port 5173 at some point since most people will probably have updated at some point.

~~Thoughts?~~

I thought about this again and explicitly adding the port could be more problematic. I think just tearing the bandaid off and notifying folks in support as well as Discords about the port change might be a better way. And again, the CLI will mention to configure targetPort in your netlify.toml if it's waiting.

➜ vite-broken-with-ntl dev/issues/vite-broken-with-ntl on  main [!?] via  v16.15.1 ➜ ntl dev ◈ Netlify Dev ◈ ◈ Ignored general context env var: LANG (defined in process) ◈ Starting Netlify Dev with Vite > vite-broken-with-ntl@0.0.0 dev > vite VITE v3.0.2 ready in 195 ms ➜ Local: http://127.0.0.1:5173/ ➜ Network: use --host to expose ⠙ Waiting for framework port 3000. This can be configured using the 'targetPort' property in the netlify.toml

I got this message with SvelteKit first changed and was not immediately aware that it couldn't find a port. It became clear eventually, but this message is stalled out. Is there a way to error out if it doesn't find one in a specific amount of time?

I agree with @brittneypostma , erroring out after a specified amount of time would be useful so it removes the perception of 'hanging' and a useful error message can help the user troubleshoot a bit better with respect to the expected port for the framework

Yeah, I think the timeout makes sense. Just wondering what makes sense in terms of time taking into account not everyone is running an M1. Maybe 30 seconds and make the message clearer? And if it fails to find it, we can add an error message like you suggested @ericapisani.

Maybe 30 seconds and make the message clearer?

I'm not sure if this is too aggressive, but I wonder if we would want to make the timeout a bit shorter (10-15 seconds) as I imagine that folks will ctrl/cmd+c the process sooner in the 30 second time frame 🤔 I'm not sure if we have any data to provide some guidance on this for us though so we can make a more informed decision

I think 10-15 seconds may be too aggressive given that not everyone is running an M1. I think if we do 30 and make the message clearer about the port we should be good alongside the port change.

Based on @seldo's comments in Slack, i.e. "People are overwhelmingly on version 3", I think it's safe to flick the switch to port 5173 and folks on older versions will get the port warning the CLI provides.

nickytonline · 2022-07-25T12:43:00Z

src/frameworks/svelte-kit.json

@@ -9,7 +9,7 @@
  },
  "dev": {
    "command": "svelte-kit dev",
-    "port": 3000,
+    "port": 5173,


@ascorbic, for svelte-kit, I know @brittneypostma ran into this the other day, so I wonder if it's OK to do the port upgrade to the new vite default port and for folks who may be using the old port, just update it? The error should be fairly clear when the CLI times out waiting for a server that is not running on port 3000.

Seeing this comment after my other one, so there is a timeout I'm just too impatient to wait for it 😅

netlify · 2022-10-18T14:04:59Z

✅ Deploy Preview for framework-info ready!

Name	Link
🔨 Latest commit	`f7a8132`
🔍 Latest deploy log	https://app.netlify.com/sites/framework-info/deploys/634eb8a4fe9356000977372b
😎 Deploy Preview	https://deploy-preview-792--framework-info.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

github-actions bot added the type: bug code to address defects in shipped code label Jul 22, 2022

nickytonline force-pushed the nickytonline/change-vite-default-port branch from de3fe87 to 90f4a10 Compare July 22, 2022 20:41

nickytonline requested review from orinokai, ascorbic and ericapisani July 22, 2022 20:41

nickytonline self-assigned this Jul 22, 2022

fix: changed the Vite port to the new Vite default port 5173

632ecd5

nickytonline force-pushed the nickytonline/change-vite-default-port branch from 90f4a10 to 632ecd5 Compare July 22, 2022 20:52

ericapisani previously approved these changes Jul 25, 2022

View reviewed changes

nickytonline commented Jul 25, 2022

View reviewed changes

nickytonline mentioned this pull request Aug 1, 2022

chore: changed qwik default port to 5173 #801

Merged

5 tasks

nickytonline mentioned this pull request Oct 4, 2022

chore: updated SvelteKit framework info #818

Merged

5 tasks

Merge branch 'main' into nickytonline/change-vite-default-port

240cd44

nickytonline dismissed ericapisani’s stale review via 240cd44 October 18, 2022 14:04

Merge branch 'main' into nickytonline/change-vite-default-port

f7a8132

nickytonline added the automerge label Oct 18, 2022

ericapisani approved these changes Oct 18, 2022

View reviewed changes

kodiakhq bot merged commit 664e6df into main Oct 18, 2022

kodiakhq bot deleted the nickytonline/change-vite-default-port branch October 18, 2022 15:19

token-generator-app bot mentioned this pull request Oct 18, 2022

chore(main): release 9.4.0 #821

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: changed the Vite port to the new Vite default port 5173 #792

fix: changed the Vite port to the new Vite default port 5173 #792

nickytonline commented Jul 22, 2022 •

edited

ascorbic commented Jul 22, 2022

nickytonline commented Jul 22, 2022 •

edited

ascorbic commented Jul 22, 2022

ericapisani left a comment

nickytonline Jul 25, 2022

nickytonline Jul 25, 2022 •

edited

brittneypostma Aug 2, 2022

ericapisani Aug 2, 2022

nickytonline Aug 2, 2022

ericapisani Aug 3, 2022

nickytonline Aug 3, 2022

nickytonline Oct 18, 2022

nickytonline Jul 25, 2022

brittneypostma Aug 2, 2022

netlify bot commented Oct 18, 2022 •

edited

fix: changed the Vite port to the new Vite default port 5173 #792

fix: changed the Vite port to the new Vite default port 5173 #792

Conversation

nickytonline commented Jul 22, 2022 • edited

Summary

ascorbic commented Jul 22, 2022

nickytonline commented Jul 22, 2022 • edited

ascorbic commented Jul 22, 2022

ericapisani left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nickytonline Jul 25, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

netlify bot commented Oct 18, 2022 • edited

✅ Deploy Preview for framework-info ready!

nickytonline commented Jul 22, 2022 •

edited

nickytonline commented Jul 22, 2022 •

edited

nickytonline Jul 25, 2022 •

edited

netlify bot commented Oct 18, 2022 •

edited