Remove tunnel flags #694
Merged
Remove tunnel flags #694
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
IN-712 Remove tunnel flags from CLI
The following flags are related to the tunnel service and are no longer needed in the CLI. These should all be removed.
In addition, the |
paulelliott
force-pushed
the
remove-tunnel-flags
branch
from
0b27cfb to
148989f
Compare
|
@ghengeveld @tmeasday Can one of you smoke test these changes locally? |
ghengeveld
reviewed
Dec 11, 2022
| const diagnostics = getInput('diagnostics'); | ||
| const debug = getInput('debug'); | ||
| const storybookPort = getInput('storybookPort'); | ||
| const storybookUrl = getInput('storybookUrl'); |
There was a problem hiding this comment.
We may want to reintroduce storybookUrl at some point (not using the tunnel) because it's quite useful for quickly testing a random Storybook published somewhere (even if not on Chromatic).
ghengeveld
approved these changes
Dec 11, 2022
action.yml
Outdated
| required: false | ||
| storybookPort: | ||
| description: 'What port is your Storybook running on (auto detected from -s, if set)' | ||
| required: false | ||
| storybookUrl: | ||
| description: 'Storybook is already running at (external) url (implies -S)' | ||
| required: false |
There was a problem hiding this comment.
I think you missed storybookUrl here.
bin-src/lib/getOptions.ts
Outdated
Comment on lines
173
to
176
| // TurboSnap requires a static build with a webpack stats file. | ||
| if (options.onlyChanged) throw new Error(invalidOnlyChanged()); | ||
|
|
||
| // Start Storybook on localhost and generate the URL to it | ||
| if (!storybookUrl) { | ||
| if (exec && !port) { | ||
| throw new Error(missingStorybookPort()); | ||
| } | ||
|
|
||
| if (!exec && (!port || !noStart)) { | ||
| // If you don't provide a port or we need to start the command, let's look up the script for it | ||
| scriptName = scriptName || 'storybook'; | ||
| const storybookScript = packageJson.scripts && packageJson.scripts[scriptName]; | ||
|
|
||
| if (!storybookScript) { | ||
| throw new Error(missingScriptName(scriptName)); | ||
| } | ||
|
|
||
| options.https = | ||
| options.https || | ||
| (getStorybookConfiguration(storybookScript, '--https') && { | ||
| cert: resolveHomeDir(getStorybookConfiguration(storybookScript, '--ssl-cert')), | ||
| key: resolveHomeDir(getStorybookConfiguration(storybookScript, '--ssl-key')), | ||
| ca: resolveHomeDir(getStorybookConfiguration(storybookScript, '--ssl-ca')), | ||
| }); | ||
|
|
||
| port = port || getStorybookConfiguration(storybookScript, '-p', '--port'); | ||
| if (!port) { | ||
| throw new Error(unknownStorybookPort(scriptName)); | ||
| } | ||
|
|
||
| if (log) log.info('', inferredOptions({ scriptName, port })); | ||
| } | ||
|
|
||
| storybookUrl = `${options.https ? 'https' : 'http'}://localhost:${port}`; | ||
| } | ||
|
|
||
| return { | ||
| ...options, | ||
| noStart, | ||
| useTunnel: true, | ||
| url: storybookUrl, | ||
| scriptName, | ||
| }; | ||
| return options; |
There was a problem hiding this comment.
These lines are unreachable after the throw above.
The check for onlyChanged was there because it's incompatible with starting Storybook rather than using a built Storybook. We can remove this check because we already check for the existence of the stats file and will just bail with missingStatsFile if it doesn't exist, and continue running the build regardless.
bin-src/lib/getOptions.ts
Outdated
Comment on lines
153
to
156
| // Build Storybook | ||
| if (storybookBuildDir) { | ||
| return { ...options }; | ||
| } |
There was a problem hiding this comment.
Suggested change
| // Build Storybook | |
| if (storybookBuildDir) { | |
| return { ...options }; | |
| } | |
| // Use prebuilt Storybook | |
| if (storybookBuildDir) { | |
| return options; | |
| } | |
| // Look for a build-storybook script to build Storybook ourselves |
bin-src/tasks/index.ts
Outdated
Comment on lines
26
to
30
| export const runPatchBuild = (runBuild: typeof runUploadBuild) => [ | ||
| prepareWorkspace, | ||
| ...runBuild, | ||
| restoreWorkspace, | ||
| ]; |
There was a problem hiding this comment.
Suggested change
| export const runPatchBuild = (runBuild: typeof runUploadBuild) => [ | |
| prepareWorkspace, | |
| ...runBuild, | |
| restoreWorkspace, | |
| ]; | |
| export const runPatchBuild = [ | |
| prepareWorkspace, | |
| ...runUploadBuild, | |
| restoreWorkspace, | |
| ]; |
bin-src/tasks/index.ts
Outdated
Comment on lines
33
to
34
| const tasks = | ||
| options.patchHeadRef && options.patchBaseRef ? runPatchBuild(runUploadBuild) : runUploadBuild; |
There was a problem hiding this comment.
Suggested change
| const tasks = | |
| options.patchHeadRef && options.patchBaseRef ? runPatchBuild(runUploadBuild) : runUploadBuild; | |
| const tasks = | |
| options.patchHeadRef && options.patchBaseRef ? runPatchBuild : runUploadBuild; |
paulelliott
force-pushed
the
remove-tunnel-flags
branch
from
d1f5e57 to
e20fc18
Compare
|
@ghengeveld Thanks for the suggestions! All updates completed. |
This includes related flags: - --storybook-cert - --storybook-key - --storybook-ca
paulelliott
force-pushed
the
remove-tunnel-flags
branch
from
e20fc18 to
50932b8
Compare
|
🚀 |
# for free
to join this conversation on GitHub.
Already have an account?
# to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This cleans out all the deprecated flags related to tunnel builds.
Fixes IN-712