CLI: Addon postinstall hooks #8700

shilman · 2019-11-04T14:13:00Z

What I did

Postinstall utilties for updating presets.js
CLI integration for sb add
Addon-docs preset update
Update documentation (after review)

How to test

I used relative-deps to test this:

npx create-react-app test-postinstall

Then I added the following to package.json:

{
  "devDependencies": {
    "@storybook/addon-actions": "^5.2.5",
    "@storybook/addon-docs": "^5.2.5",
    "@storybook/addon-links": "^5.2.5",
    "@storybook/addons": "^5.2.5",
    "@storybook/cli": "^5.2.5",
    "@storybook/react": "^5.2.5",
    "relative-deps": "^0.2.0"
  },
  "relativeDependencies": {
    "@storybook/cli": "../storybook/lib/cli",
    "@storybook/addon-docs": "../storybook/addons/docs",
    "@storybook/postinstall": "../storybook/lib/postinstall"
  }
}

Then I ran I installed the dependencies:

yarn # install deps
yarn relative-deps

Then I added docs and verified the output in .storybook/presets:

yarn sb add docs

NOTE: For some reason, isOfficialAddon is false, so I had to stub that out. It's possible that the calculation function is broken? Or that using relative-deps screws things up? Or that npm is down?

vercel · 2019-11-04T14:13:04Z

This pull request is being automatically deployed with ZEIT Now (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://zeit.co/storybook/monorepo/nqi8ry7ox
🌍 Preview: https://monorepo-git-8577-addons-postinstall.storybook.now.sh

ndelangen

So you want addons to be capable of rewriting a user's presets.js / configuration after being added by the CLI.

This PR scopes it pretty narrowly to "just preset.js" and only a single codemod on that exact file.

I wonder if we should do the following:

write a package that exports a function "to run a codemod on a storybook config file" on.

// some-addon/postinstall.js
import { runCodeMod, addPreset, addAddon } from '@storybook/codemod';

// run a codemod with a function
runCodeMod('preset', ({ api, root }) => {}));

// run a very easy codemod just to add a preset
addPreset(['typescript', 'sass']);

// run a very easy codemod to add a addon ref in manager
addAddon('docs');

// users can even decide to take matters into their own hands, and just do some JS in node here

Let users use our predefined functions to make it easy for them, but give them the control they'd need to do powerful stuff.

ndelangen · 2019-11-04T16:57:49Z

lib/cli/lib/add.js

+    skipMsg = 'unofficial addon';
+  } else {
+    const presetsCodemod = require.resolve(
+      `${getPackageName(addonName, isOfficialAddon)}/postinstall/presets.js`


Are you expecting other files other then preset.js here?
Why not just make it index.js?

Yeah will add support for other files in future

ndelangen · 2019-11-04T16:58:54Z

lib/cli/lib/add.js

+      `${getPackageName(addonName, isOfficialAddon)}/postinstall/presets.js`
+    );
+    if (fs.existsSync(presetsCodemod)) {
+      if (fs.existsSync('.storybook')) {


I guess there's no way to know if there's a non-standard configDir?

I think the only way is to ok infer it from package.json which is what chromatic does

ndelangen · 2019-11-04T17:02:33Z

lib/cli/lib/add.js

+        if (!fs.existsSync(presetsFile)) {
+          fs.writeFileSync(presetsFile, '', 'utf8');
+        }
+        spawnSync('npx', ['jscodeshift', '-t', presetsCodemod, presetsFile], {


So the postinstall is actually a "post-add-modemod". It's limited to be exactly a codemod for presets.js only?

I THINK presets.js can be a .ts file too.

Nope AFAIK it can't

ndelangen · 2019-11-04T17:12:25Z

lib/addons/src/postinstall/codemods.ts

+  api: any;
+}
+
+export function addPreset(preset: string, presetOptions: any, { api, root }: PostinstallContext) {


Why is this part of addons? It seems not meant to be part of either the manager or preview's code at all?

Didn't want add-ons to have to take that dependency. You'll notice that this uses dependency injection to avoid needing any extra deps

addons/docs/postinstall/presets.js

ndelangen

So you want addons to be capable of rewriting a user's presets.js / configuration after being added by the CLI.

This PR scopes it pretty narrowly to "just preset.js" and only a single codemod on that exact file.

I wonder if we should do the following:

write a package that exports a function "to run a codemod on a storybook config file" on.

// some-addon/postinstall.js
import { runCodeMod, addPreset, addAddon } from '@storybook/codemod';

// run a codemod with a function
runCodeMod('preset', ({ api, root }) => {}));

// run a very easy codemod just to add a preset
addPreset(['typescript', 'sass']);

// run a very easy codemod to add a addon ref in manager
addAddon('docs');

// users can even decide to take matters into their own hands, and just do some JS in node here

Let users use our predefined functions to make it easy for them, but give them the control they'd need to do powerful stuff.
Run this postinstall.js | postinstall.ts when the package is added via the CLI.

addAddon would ensure the addon is added once to the entry of manager see: f8e6f31#diff-ec8cd75f31bce9dc44ea59d186747551R3-R21

addpreset would do the exact same as above, but obviously to the presets property.

We can deduplicate at runtime too, to keep the codemods even simpler?

ndelangen

whoops duped

shilman · 2019-11-04T20:45:13Z

@ndelangen I considered that, but the tradeoff here is addon package size & security. In the current PR, jscodeshift is a dep of the CLI. If we add it to the addon, that means that every addon that uses this functionality will need to depend on jscodeshift. Currently it doesn't add any weight to the addon.

The addon has the ability to run any codemod it wants on .storybook/presets.js. And in the near future, will get the same for addons.[tj]s & config.[tj]s, which is pretty flexible.

tmeasday

Approach seems reasonable to me.

shilman added 3 commits November 4, 2019 22:06

Addons API: Utils for sb add postinstall hooks

fb6b4d6

CLI: Postinstall hooks for addons

2cd2e8b

Addon-docs: Postinstall hooks to update presets

a39724c

shilman added feature request api: addons cli addon: docs labels Nov 4, 2019

shilman requested a review from tmeasday November 4, 2019 14:13

shilman requested review from ndelangen, stijnkoopal and theinterned as code owners November 4, 2019 14:13

shilman added this to the 5.3.0 milestone Nov 4, 2019

Fix sb add postinstall skip message

0965687

vercel bot deployed to staging November 4, 2019 14:17 View deployment

shilman self-assigned this Nov 4, 2019

shilman requested a review from igor-dv November 4, 2019 14:28

ndelangen approved these changes Nov 4, 2019

View reviewed changes

ndelangen requested changes Nov 4, 2019

View reviewed changes

tmeasday approved these changes Nov 6, 2019

View reviewed changes

Merge branch 'next' into 8577-addons-postinstall

a001fe4

vercel bot deployed to staging November 11, 2019 15:32 View deployment

Create postinstall library for addons

4c9b100

vercel bot deployed to staging November 12, 2019 10:02 View deployment

shilman changed the title ~~Addons postinstall hooks~~ CLI: Addon postinstall hooks Nov 12, 2019

Merge branch 'next' into 8577-addons-postinstall

5b65d02

vercel bot had a problem deploying to staging November 12, 2019 14:01 Failure

Update package.json

08e8964

vercel bot deployed to staging November 12, 2019 14:28 View deployment

Remove debugging comment 🙈

c5c6d41

vercel bot deployed to staging November 12, 2019 15:11 View deployment

Merge branch 'next' into 8577-addons-postinstall

493c1cc

vercel bot had a problem deploying to staging November 14, 2019 00:59 Failure

Update package.json

be8dc3e

vercel bot deployed to staging November 14, 2019 02:52 View deployment

Poinstall: Preset codemod to single quotes

e75cfdc

vercel bot temporarily deployed to staging November 14, 2019 03:33 Inactive

Update postinstall codemods test & snapshots

a2006b9

vercel bot requested a deployment to staging November 14, 2019 03:41 Pending

shilman merged commit cbc16f5 into next Nov 14, 2019

shilman deleted the 8577-addons-postinstall branch November 14, 2019 03:41

vercel bot deployed to staging November 14, 2019 03:48 View deployment

shilman mentioned this pull request Nov 14, 2019

CLI: Postinstall hooks for addons #8577

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CLI: Addon postinstall hooks #8700

CLI: Addon postinstall hooks #8700

shilman commented Nov 4, 2019 •

edited

Loading

vercel bot commented Nov 4, 2019 •

edited

Loading

ndelangen left a comment

ndelangen Nov 4, 2019

shilman Nov 4, 2019

ndelangen Nov 4, 2019

shilman Nov 4, 2019

ndelangen Nov 4, 2019

shilman Nov 4, 2019

ndelangen Nov 4, 2019

shilman Nov 4, 2019

ndelangen left a comment •

edited

Loading

ndelangen left a comment •

edited

Loading

shilman commented Nov 4, 2019

tmeasday left a comment

CLI: Addon postinstall hooks #8700

CLI: Addon postinstall hooks #8700

Conversation

shilman commented Nov 4, 2019 • edited Loading

What I did

How to test

vercel bot commented Nov 4, 2019 • edited Loading

ndelangen left a 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

Choose a reason for hiding this comment

ndelangen left a comment • edited Loading

Choose a reason for hiding this comment

ndelangen left a comment • edited Loading

Choose a reason for hiding this comment

shilman commented Nov 4, 2019

tmeasday left a comment

Choose a reason for hiding this comment

shilman commented Nov 4, 2019 •

edited

Loading

vercel bot commented Nov 4, 2019 •

edited

Loading

ndelangen left a comment •

edited

Loading

ndelangen left a comment •

edited

Loading