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

settings: npm command for run storybook #571

Closed
kvart714 opened this issue Sep 26, 2022 · 11 comments · Fixed by #803
Closed

settings: npm command for run storybook #571

kvart714 opened this issue Sep 26, 2022 · 11 comments · Fixed by #803
Labels
enhancement New feature or request

Comments

@kvart714
Copy link
Contributor

First of all: I just love this extension! 😄

According to the official documentation:
https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#angular13

Starting from Angular 13 i have to use angular.json to setup storybook
So my command to run sotorybook is ng run my-default-project:storybook

Which is why I have to use the "external server" option and run the storybook manually

It would be cool if in the settings you could just specify the npm command to start the internal storybook server!

@joshbolduc joshbolduc added the enhancement New feature or request label Sep 27, 2022
@joshbolduc
Copy link
Owner

As a workaround, I think these settings should work:

{
  "storyExplorer.server.internal.storybookBinaryPath": "./node_modules/.bin/ng",
  "storyExplorer.server.internal.commandLineArgs": [
    "run",
    "my-default-project:storybook"
  ]
}

(Depending on your project/environment, you may need to adjust the path to ng.)

@kvart714
Copy link
Contributor Author

nice idea, thanks!
but I needed to use ng.cmd
./node_modules/.bin/ng.cmd worked for me

@kvart714
Copy link
Contributor Author

kvart714 commented Sep 28, 2022

it is also possible to run an npm command (what I originally wanted 😄). a little more generic

{
    "storyExplorer.server.internal.storybookBinaryPath": "C:/Program Files/nodejs/npm.cmd",
    "storyExplorer.server.internal.commandLineArgs": ["run", "storybook"]
}

@kvart714
Copy link
Contributor Author

I have configured tasks.json and launch.json files for debug storybook. But this starts a different task than the extension.

maybe it would be better to run a not npm/executable command from an extension, but custom existing tasks from tasks.json? 🤔

I found a solution for myself:

// launch.json
{
    "name": "storybook",
    "type": "chrome",
    "request": "launch",
    "port": 9222,
    "preLaunchTask": "run storybook",
    "url": "http://localhost:6006",
    "urlFilter": "http://localhost:6006",
    "userDataDir": false,
    "cleanUp": "onlyTab"
}
// tasks.json
{
    // instead of running storybrook command, I run the extension command
    "command": "${command:storyExplorer.startStorybookServer}",
    "label": "run storybook",
    "isBackground": false
}

this works as expected: if the server is already running, then debug starts instantly
but it doesn't work with "isBackground": true and with problemMatcher.

@joshbolduc
Copy link
Owner

I think it might be useful to have some configuration here: launch the storybook script directly, run an npm script, or launch an existing task. I could see situations where any of those options could be useful.

joshbolduc added a commit that referenced this issue Feb 20, 2023
Add support for new launch strategies for launching a Storybook
developer server. This allows the server to be launched via direct
invocation (`storybook` for Storybook 7 or `start-storybook` for
Storybook 6), npm script, VS Code task, or custom script.

The existing settings for configuring the `start-storybook` binary path
and arguments are deprecated in favor of new options.

Fixes #571
Fixes #605
joshbolduc added a commit that referenced this issue Feb 20, 2023
Add support for new launch strategies for launching a Storybook
developer server. This allows the server to be launched via direct
invocation (`storybook` for Storybook 7 or `start-storybook` for
Storybook 6), npm script, VS Code task, or custom script.

The existing settings for configuring the `start-storybook` binary path
and arguments are deprecated in favor of new options.

Fixes #571
Fixes #605
joshbolduc added a commit that referenced this issue Feb 20, 2023
Add support for new launch strategies for launching a Storybook
developer server. This allows the server to be launched via direct
invocation (`storybook` for Storybook 7 or `start-storybook` for
Storybook 6), npm script, VS Code task, or custom script.

The existing settings for configuring the `start-storybook` binary path
and arguments are deprecated in favor of new options.

Fixes #571
Fixes #605
@joshbolduc
Copy link
Owner

With #803, launching a Storybook server via an npm script is now supported. There are a handful of new options now documented in the README that you can check out if you want to customize further. But if you were using a workaround with storyExplorer.server.internal.storybookBinaryPath and storyExplorer.server.internal.commandLineArgs, you can remove them now and instead set storyExplorer.server.internal.launchStrategy to npm. (I'm thinking about whether this should actually be the default, and start-storybook/storybook become fallbacks instead--feedback welcome!)

One other new launch option is launching an existing VS Code task, which should work as long as the task uses process execution under the hood. (That said, if you tell Story Explorer to run a task that runs a Story Explorer command to start the server, which will run a task that runs a Story Explorer command to start the server... that probably won't work. But it should for other tasks, or other launch strategies. 🙂)

Sidenote: I haven't really done much to improve debugging workflows here, but I think there's a fair bit that could be done in that space. Flexibility in launch strategies is a small improvement, but I think there's more opportunity--if you have ideas or suggestions here, feel free to open a new issue to explore the space a bit more. This has been broached vaguely before in #383.

@kvart714
Copy link
Contributor Author

First of all, I want to say thank you! 🙂
I've checked npm strategy - it works perfect!

I also tried the task strategy
But VS Code and Extension run 2 different tasks with the same label (or 2 instances of the same task, you can see in the screenshot)
Anyway one of these tasks will fail because they both try to use the same port

If I run task twice from VS Code I get a message The task is already active. Terminate task/Restart task (also in the screenshot). So VS Code can understand when a task is running. But your task for VS Code is another one.

I've checked the first random extension:
It use a method vscode.tasks.fetchTasks() to get tasks and vscode.tasks.executeTask(task) to runs one of them.
And in this case I got default behaviour with The task is already active message

image

@kvart714
Copy link
Contributor Author

one more point:
actually label is optional field for tasks
in this case vscode uses {type}: {script} for label
e g I can use "preLaunchTask": "npm: storybook" in launch.json for run task without label

image

@joshbolduc
Copy link
Owner

Thanks for pointing out the relaunch behavior--what you describe should happen is in fact what I intended, but it turns out there was a bug that prevented it from working as expected. I just merged #812, which should fix that.

FWIW, there are some cases where the extension can't reuse an existing task the way things are, including:

  • if the other task had started before the extension was activated. This is because the extension uses the process ID of the other task to figure out what port the Storybook server is using, and VS Code only makes that information available via an event that fires when the task starts. So even if the extension knows the task is running, if it doesn't know its process ID because it hadn't started up in time to catch it when the task launched, it'll launch a new one.
  • if you reload your window and your existing tasks are reattached, VS Code itself doesn't seem to always recognize them as running tasks, so even if the above problems were solved, the tasks don't show up as running tasks in the first place, so we have to proceed as if they aren't running.

These scenarios are hopefully rare, but they could happen.

The extra launch ought to succeed in general, even if there's a port conflict--are you using Storybook 7 by chance? If so, there's a bug in the current beta which causes the port conflict prompt you're seeing there. (If not, then the way I'm doing it might not be working the way I thought. 🤔)

actually label is optional field for tasks

Yes, I think this is correct. In that case, AFAICT, the "label" is effectively derived from the other properties. So you could target that task using storybook as the label (and npm as the type, if you needed to disambiguate).

It's not super intuitive--I had to test that out to figure out what the right values were. I am matching on what VS Code internally presents as the task's name, which also isn't a required (or allowed) property of a task in tasks.json. I don't think there's any generic property that is guaranteed to exist on an arbitrary task, but best I can tell label is always at least supported and takes precedence if specified. It's not very clear, but it might be the least unclear option. There may be better ways to document it, though.

@kvart714
Copy link
Contributor Author

kvart714 commented Feb 22, 2023

are you using Storybook 7 by chance?

no, we are currently using SB 6

This is because the extension uses the process ID of the other task to figure out what port the Storybook server is using

I debugged your code and figured out how it works.
I also understand why sometimes the extension cannot start the storybook

for experiment I duplicated pollForPort call
first time I got port 65302
a little later I got port 59117
but in fact the storybook is running on 6006!

I have not yet been able to reproduce the problem stably 😔

image

@kvart714
Copy link
Contributor Author

Sorry for I continued on this thread 🙂
So I created Pull Request

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants