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

Added an upgrade mode to getstorybook #1146

Merged
merged 2 commits into from
May 30, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion MIGRATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,20 @@ You can find the guide to upgrading your webpack config [on webpack.js.org](http

### Packages renaming

All our packages have been renamed and published to npm as version 3.0.0.
All our packages have been renamed and published to npm as version 3.0.0 under the `@storybook` namespace.

To update your app to use the new package names, you can use the cli:

```bash
npm install --global @storybook/cli

# if you run this inside a v2 app, it should perform the necessary codemods.
getstorybook
```

#### Details

If the above doesn't work, or you want to make the changes manually, the details are below:

> We have adopted the same versioning strategy as have been adopted by babel, jest and apollo.
> It's a strategy best suited for ecosystem type tools, which consist of many separately installable features / packages.
Expand Down
8 changes: 8 additions & 0 deletions lib/cli/bin/generate.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,14 @@ switch (projectType) {
// Add a new line for the clear visibility.
logger.log();
break;

case types.UPDATE_PACKAGE_ORGANIZATIONS:
require('../generators/UPDATE_PACKAGE_ORGANIZATIONS')
.then(r => null) // commmandLog doesn't like to see output
.then(commandLog('Upgrading your project to the new storybook packages.'))
.then(end);
break;

case types.REACT_SCRIPTS:
require('../generators/REACT_SCRIPTS')
.then(commandLog('Adding storybook support to your "Create React App" based project'))
Expand Down
51 changes: 51 additions & 0 deletions lib/cli/generators/UPDATE_PACKAGE_ORGANIZATIONS/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
const helpers = require('../../lib/helpers');
const latestVersion = require('latest-version');
const spawn = require('child-process-promise').spawn;
const path = require('path');

const packageNames = require('@storybook/codemod').packageNames;

function updatePackage(devDependencies, oldName, newName) {
if (devDependencies[oldName]) {
return latestVersion(newName).then(version => {
delete devDependencies[oldName];
devDependencies[newName] = version;
});
} else {
return Promise.resolve(null);
}
}

function updatePackageJson() {
const packageJson = helpers.getPackageJson();
const { devDependencies } = packageJson;

return Promise.all(
Object.keys(packageNames).map(oldName => {
const newName = packageNames[oldName];
return updatePackage(devDependencies, oldName, newName);
})
).then(() => {
if (!devDependencies['@storybook/react'] && !devDependencies['@storybook/react-native']) {
throw new Error('Expected to find `@kadira/[react-native]-storybook` in devDependencies');
}
helpers.writePackageJson(packageJson);
});
}

function updateSourceCode() {
const jscodeshiftPath = path.dirname(require.resolve('jscodeshift'));
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a reliable way to get the path to the compiled package? jscodeshift doesn't appear to offer its commandline functionality via an API

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you found this method to be unreliable?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, but it feels a little hacky. For instance it hard codes the path of the binary into this file (rather than npm run jscodeshift which uses the bin field of jscodeshift's package.json).

const jscodeshiftCommand = path.join(jscodeshiftPath, 'bin', 'jscodeshift.sh');

const codemodPath = path.join(
path.dirname(require.resolve('@storybook/codemod')),
'transforms',
'update-organisation-name.js'
);

const args = ['-t', codemodPath, '--silent', '--ignore-pattern', '"node_modules|dist"', '.'];

return spawn(jscodeshiftCommand, args, { stdio: 'inherit' });
}

module.exports = updatePackageJson().then(updateSourceCode);
16 changes: 16 additions & 0 deletions lib/cli/lib/detect.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,22 @@ module.exports = function detect(options) {
return types.UNDETECTED;
}

if (!options.force && packageJson.devDependencies) {
if (
packageJson.devDependencies['@storybook/react'] ||
packageJson.devDependencies['@storybook/react-native']
) {
return types.ALREADY_HAS_STORYBOOK;
}

if (
packageJson.devDependencies['@kadira/storybook'] ||
packageJson.devDependencies['@kadira/react-native-storybook']
) {
return types.UPDATE_PACKAGE_ORGANIZATIONS;
}
}

if (
!options.force &&
packageJson.devDependencies &&
Expand Down
1 change: 1 addition & 0 deletions lib/cli/lib/project_types.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,5 @@ module.exports = {
REACT_PROJECT: 'REACT_PROJECT',
WEBPACK_REACT: 'WEBPACK_REACT',
ALREADY_HAS_STORYBOOK: 'ALREADY_HAS_STORYBOOK',
UPDATE_PACKAGE_ORGANIZATIONS: 'UPDATE_PACKAGE_ORGANIZATIONS',
};
3 changes: 3 additions & 0 deletions lib/cli/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,12 @@
},
"homepage": "https://github.com/storybooks/storybook/tree/master/lib/cli",
"dependencies": {
"@storybook/codemod": "^3.0.0-rc.2",
"chalk": "^1.1.3",
"child-process-promise": "^2.2.1",
"commander": "^2.9.0",
"cross-spawn": "^5.0.1",
"jscodeshift": "^0.3.30",
"json5": "^0.5.1",
"merge-dirs": "^0.2.1",
"opencollective": "^1.0.3",
Expand Down
4 changes: 2 additions & 2 deletions lib/codemod/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ From the directory where you installed both `jscodeshift` and `@storybook/codemo
Example:

```sh
./node_modules/.bin/jscodeshift -t ./node_modules/@storybook/codemod/dist/update-organisation-name.js . --ignore-pattern "node_modules|dist"
./node_modules/.bin/jscodeshift -t ./node_modules/@storybook/codemod/dist/transforms/update-organisation-name.js . --ignore-pattern "node_modules|dist"
```

Explanation:
Expand All @@ -43,7 +43,7 @@ Explanation:
Updates package names in imports to migrate to the new package names of storybook.

```sh
./node_modules/.bin/jscodeshift -t ./node_modules/@storybook/codemod/dist/update-organisation-name.js . --ignore-pattern "node_modules|dist"
./node_modules/.bin/jscodeshift -t ./node_modules/@storybook/codemod/dist/transforms/update-organisation-name.js . --ignore-pattern "node_modules|dist"
```

There's a mapping of paths we replace but this example explains the gist of it:
Expand Down
4 changes: 2 additions & 2 deletions lib/codemod/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,12 @@
"type": "git",
"url": "git+https://github.com/storybooks/storybook.git"
},
"main": "src/index.js",
"main": "dist/index.js",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if the old main field was a mistake or if I'm missing something

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This package is never ever required by anything. This explains why this bug was never found.

"dependencies": {
"jscodeshift": "^0.3.30"
},
"scripts": {
"prepublish": "node ../../scripts/prepublish.js && mv dist/transforms/* dist"
"prepublish": "node ../../scripts/prepublish.js"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ndelangen you should perhaps check this change.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added this to shorten the paths, if this is removed the manual needs to be updated

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated the README for the package, was there anything else that needed to change?

},
"devDependencies": {
"shelljs": "^0.7.7"
Expand Down
5 changes: 4 additions & 1 deletion lib/codemod/src/index.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
/* eslint import/prefer-default-export: "off" */

export { default as updateOrganisationName } from './transforms/update-organisation-name';
export {
default as updateOrganisationName,
packageNames,
} from './transforms/update-organisation-name';
45 changes: 23 additions & 22 deletions lib/codemod/src/transforms/update-organisation-name.js
Original file line number Diff line number Diff line change
@@ -1,28 +1,29 @@
export const packageNames = {
'@kadira/react-storybook-decorator-centered': '@storybook/addon-centered',
'@kadira/storybook-addons': '@storybook/addons',
'@kadira/storybook-addon-actions': '@storybook/addon-actions',
'@kadira/storybook-addon-comments': '@storybook/addon-comments',
'@kadira/storybook-addon-graphql': '@storybook/addon-graphql',
'@kadira/storybook-addon-info': '@storybook/addon-info',
'@kadira/storybook-addon-knobs': '@storybook/addon-knobs',
'@kadira/storybook-addon-links': '@storybook/addon-links',
'@kadira/storybook-addon-notes': '@storybook/addon-notes',
'@kadira/storybook-addon-options': '@storybook/addon-options',
'@kadira/storybook-channels': '@storybook/channels',
'@kadira/storybook-channel-postmsg': '@storybook/channel-postmessage',
'@kadira/storybook-channel-websocket': '@storybook/channel-websocket',
'@kadira/storybook-ui': '@storybook/ui',
'@kadira/react-native-storybook': '@storybook/react-native',
'@kadira/react-storybook': '@storybook/react',
'@kadira/getstorybook': '@storybook/cli',
'@kadira/storybook': '@storybook/react',
storyshots: '@storybook/addon-storyshots',
getstorybook: '@storybook/cli',
};

export default function transformer(file, api) {
const j = api.jscodeshift;

const packageNames = {
'@kadira/react-storybook-decorator-centered': '@storybook/addon-centered',
'@kadira/storybook-addons': '@storybook/addons',
'@kadira/storybook-addon-actions': '@storybook/addon-actions',
'@kadira/storybook-addon-comments': '@storybook/addon-comments',
'@kadira/storybook-addon-graphql': '@storybook/addon-graphql',
'@kadira/storybook-addon-info': '@storybook/addon-info',
'@kadira/storybook-addon-knobs': '@storybook/addon-knobs',
'@kadira/storybook-addon-links': '@storybook/addon-links',
'@kadira/storybook-addon-notes': '@storybook/addon-notes',
'@kadira/storybook-addon-options': '@storybook/addon-options',
'@kadira/storybook-channels': '@storybook/channels',
'@kadira/storybook-channel-postmsg': '@storybook/channel-postmessage',
'@kadira/storybook-channel-websocket': '@storybook/channel-websocket',
'@kadira/storybook-ui': '@storybook/ui',
'@kadira/react-native-storybook': '@storybook/react-native',
'@kadira/react-storybook': '@storybook/react',
'@kadira/getstorybook': '@storybook/cli',
'@kadira/storybook': '@storybook/react',
storyshots: '@storybook/addon-storyshots',
getstorybook: '@storybook/cli',
};
const packageNamesKeys = Object.keys(packageNames);

/**
Expand Down