-
Notifications
You must be signed in to change notification settings - Fork 103
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
Use a random port for GraphiQL when the default one is not available #3579
Conversation
Thanks for your contribution! Depending on what you are working on, you may want to request a review from a Shopify team:
|
Coverage report
Show files with reduced coverage 🔻
Test suite run success1593 tests passing in 741 suites. Report generated by 🧪jest coverage report action from 01fbd0e |
I'm not sure this is a good idea. Certain features of GraphiQL (query history, session recovery) break if you switch ports, so I'm thinking users should have to do this manually. |
I know, but even without history, GrahpiQL keeps working and it's not the main purpose of the dev command. It must be annoying for people having the default port already in use, or when they want to dev two different apps at the same time... What if we show a warning when the port is random? Something like:
|
What is the use case for running |
@nickwesselman I guess the main use case would be to work on two different projects, being able to switch without stopping the dev command. I think there are no other side effects, I've done it many times until we added GrahpiQL. |
Differences in type declarationsWe detected differences in the type declarations generated by Typescript for this branch compared to the baseline ('main' branch). Please, review them to ensure they are backward-compatible. Here are some important things to keep in mind:
New type declarationsWe found no new type declarations in this PR Existing type declarationspackages/cli-kit/dist/public/node/tcp.d.ts@@ -1,10 +1,11 @@
/**
* Returns an available port in the current environment.
*
+ * @param preferredPort - Number of the preferred port to be used if available.
* @returns A promise that resolves with an availabe port.
* @example
*/
-export declare function getAvailableTCPPort(): Promise<number>;
+export declare function getAvailableTCPPort(preferredPort?: number): Promise<number>;
/**
* Checks if a port is available.
*
|
WHY are these changes introduced?
Fixes #3578
Since GraphiQL was added, you can't run
dev
multiple times simultaneously without specifying a different GraphiQL port, as we always use the same default value (3457). When you try to run dev a second time, it raises an error:WHAT is this pull request doing?
If the default port is in use, find an available one instead of raising an error. In that case, a warning is shown:
How to test your changes?
Run
p shopify app dev
in two tabs and ensure that GraphiQL works in both placesMeasuring impact
How do we know this change was effective? Please choose one:
Checklist
dev
ordeploy
have been reflected in the internal flowchart.