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

3.0.0 migration assistance : codemod, update installation and usage instructions #1093

Merged
merged 4 commits into from
May 27, 2017

Conversation

aaronmcadam
Copy link
Contributor

@aaronmcadam aaronmcadam commented May 22, 2017

I noticed that the codemod script name is wrong.

TODO

  • Verify the best practice for installing codemods: local repo clone or global registry?
  • Write usage guide for codemods
  • Record screen video of usage
  • Write manual migration guide

@shilman
Copy link
Member

shilman commented May 22, 2017

@aaronmcadam Can you add instructions for how to apply this to a whole project from start to finish?

  1. do i have to install this as a dev dependency for my project?
  2. how is there some typical find *.js | xargs ... command to run this over my entire project?

@aaronmcadam
Copy link
Contributor Author

  1. ...

You need to be careful to ignore your dist or other bundle output. You can use your .gitignore file for this.

Can you try jscodeshift -t lib/codemod/src/transforms/update-organisation-name.js --ignore-config .gitignore path/to/your/project?

We also need to check whether we have to tell users to clone the repo, like react-codemod, or like the docs say, install the package and register it with jscodeshift somehow.

@aaronmcadam aaronmcadam changed the title docs(codemod): fix a few typos docs(codemod): update installation and usage instructions May 22, 2017
@ndelangen
Copy link
Member

ndelangen commented May 22, 2017

@aaronmcadam Do you think you can demo this?

We could create a video like so:

  1. create a folder
  2. start CRA in it
  3. start 2.x.x storybook in it
  4. run codemods
  5. cleanup
  6. run cra & storybook
  7. cake

I need help with step 4.

@shilman
Copy link
Member

shilman commented May 23, 2017

@aaronmcadam how hard would it be to add this to getstorybook? Or is that a terrible idea? 😬

you run it in your project:

  • if storybook is not installed => current behavior
  • if storybook is already installed
    • if storybook up to date => do nothing
    • if storybook is not up to date => run the codemod in *.js (smartly somehow)

@ndelangen
Copy link
Member

I have big plans for the cli, and that would be one of them @shilman

I actually want to build something like this (eventually):

storybook start   # starts and opens storybook 
storybook start --public  # starts and create a temp public url to share
storybook build  # builds to static site
storybook deploy  # builds to static site & deploy to heroku
storybook help <searchterm>  # search our documentation 
storybook migrate  # run codemods to latest version

@shilman
Copy link
Member

shilman commented May 23, 2017

@aaronmcadam it's not working for me. here's my steps:

  505  yarn create react-app testapp
  506  cd testapp/
  507  yarn add getstorybook
  508  ./node_modules/.bin/getstorybook 
  509 cd ../storybook
  510 jscodeshift -t ./lib/codemod/src/transforms/update-organisation-name.js --ignore-config .gitignore ../testapp/

I'm getting errors that look like this:

MBP:storybook shilman$ jscodeshift -t ./lib/codemod/src/transforms/update-organisation-name.js --ignore-config .gitignore ../testapp/
Processing 17179 files... 
Spawning 3 workers...
Sending 50 files to free worker...
Sending 50 files to free worker...
Sending 50 files to free worker...
 ERR ../testapp/node_modules/asn1.js/test.js Transformation error
SyntaxError: Unexpected token (6:0)
    at Parser.pp.raise (/Users/shilman/.config/yarn/global/node_modules/babel-core/node_modules/babylon/lib/parser/location.js:24:13)
    at Parser.pp.unexpected (/Users/shilman/.config/yarn/global/node_modules/babel-core/node_modules/babylon/lib/parser/util.js:82:8)
    at Parser.pp.parseExprAtom (/Users/shilman/.config/yarn/global/node_modules/babel-core/node_modules/babylon/lib/parser/expression.js:425:12)
    at Parser.parseExprAtom (/Users/shilman/.config/yarn/global/node_modules/babel-core/node_modules/babylon/lib/plugins/jsx/index.js:412:22)
    at Parser.pp.parseExprSubscripts (/Users/shilman/.config/yarn/global/node_modules/babel-core/node_modules/babylon/lib/parser/expression.js:236:19)
    at Parser.pp.parseMaybeUnary (/Users/shilman/.config/yarn/global/node_modules/babel-core/node_modules/babylon/lib/parser/expression.js:217:19)
    at Parser.pp.parseExprOps (/Users/shilman/.config/yarn/global/node_modules/babel-core/node_modules/babylon/lib/parser/expression.js:163:19)
    at Parser.pp.parseMaybeConditional (/Users/shilman/.config/yarn/global/node_modules/babel-core/node_modules/babylon/lib/parser/expression.js:145:19)
    at Parser.pp.parseMaybeAssign (/Users/shilman/.config/yarn/global/node_modules/babel-core/node_modules/babylon/lib/parser/expression.js:112:19)
    at Parser.pp.parseVar (/Users/shilman/.config/yarn/global/node_modules/babel-core/node_modules/babylon/lib/parser/statement.js:493:24)
Sending 50 files to free worker...
Sending 50 files to free worker...
Sending 50 files to free worker...
Sending 50 files to free worker...
Sending 50 files to free worker...
Sending 50 files to free worker...
 ERR ../testapp/node_modules/errno/cli.js Transformation error
SyntaxError: 'return' outside of function (8:2)
    at Parser.pp.raise (/Users/shilman/.config/yarn/global/node_modules/babel-core/node_modules/babylon/lib/parser/location.js:24:13)
    at Parser.pp.parseReturnStatement (/Users/shilman/.config/yarn/global/node_modules/babel-core/node_modules/babylon/lib/parser/statement.js:275:10)
    at Parser.pp.parseStatement (/Users/shilman/.config/yarn/global/node_modules/babel-core/node_modules/babylon/lib/parser/statement.js:89:19)
    at Parser.parseStatement (/Users/shilman/.config/yarn/global/node_modules/babel-core/node_modules/babylon/lib/plugins/flow.js:655:22)
    at Parser.pp.parseIfStatement (/Users/shilman/.config/yarn/global/node_modules/babel-core/node_modules/babylon/lib/parser/statement.js:268:26)
    at Parser.pp.parseStatement (/Users/shilman/.config/yarn/global/node_modules/babel-core/node_modules/babylon/lib/parser/statement.js:87:19)
    at Parser.parseStatement (/Users/shilman/.config/yarn/global/node_modules/babel-core/node_modules/babylon/lib/plugins/flow.js:655:22)
    at Parser.pp.parseTopLevel (/Users/shilman/.config/yarn/global/node_modules/babel-core/node_modules/babylon/lib/parser/statement.js:30:21)
    at Parser.parse (/Users/shilman/.config/yarn/global/node_modules/babel-core/node_modules/babylon/lib/parser/index.js:70:17)
    at Object.parse (/Users/shilman/.config/yarn/global/node_modules/babel-core/node_modules/babylon/lib/index.js:45:50)

And the resulting files look something like this (note this is from a different app, but the pattern is the same):

import { configure } from undefined;

import undefined

const req = require.context('../src', true, /.story.js$/);

function loadStories() {
  req.keys().forEach(filename => req(filename));
}

configure(loadStories, module);

Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

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

Need help! See my comment above 😄

@codecov
Copy link

codecov bot commented May 23, 2017

Codecov Report

Merging #1093 into master will decrease coverage by 0.02%.
The diff coverage is 66.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1093      +/-   ##
==========================================
- Coverage   12.86%   12.84%   -0.03%     
==========================================
  Files         198      198              
  Lines        4501     4540      +39     
  Branches      715      718       +3     
==========================================
+ Hits          579      583       +4     
- Misses       3289     3322      +33     
- Partials      633      635       +2
Impacted Files Coverage Δ
...codemod/src/transforms/update-organisation-name.js 43.33% <66.66%> (-56.67%) ⬇️
...rganisation-name/update-organisation-name.input.js 0% <0%> (ø) ⬆️
...ganisation-name/update-organisation-name.output.js 0% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4ef92e6...c6bddac. Read the comment docs.

@aaronmcadam
Copy link
Contributor Author

Hey @shilman judging from that output, it looks like the .gitignore isn't ignoring node_modules. I'll try this later though and get back to you

@ndelangen ndelangen added this to the v3.0.0 milestone May 26, 2017
@ndelangen ndelangen self-assigned this May 26, 2017
@ndelangen ndelangen changed the title docs(codemod): update installation and usage instructions 3.0.0 migration assistance : codemod, update installation and usage instructions May 26, 2017
@ndelangen ndelangen added documentation maintenance User-facing maintenance tasks labels May 26, 2017
@ndelangen ndelangen force-pushed the am-fix-codemod-docs branch from 5bea2d9 to d0ea916 Compare May 26, 2017 19:07
@ndelangen
Copy link
Member

Here's the implementation of the codemod:
https://astexplorer.net/#/gist/f2d0b42c37e4556a4f816892be6ca402/ad64d6387a68461556dd57d1cc3d4124425f1573

Give it a test

@shilman
Copy link
Member

shilman commented May 27, 2017

@ndelangen I am still having problems running this on my project. Can you please test it on a from-scratch project that is not part of the monorepo and tell me if it works for you. Here are my repro steps, updated per your instructions:

  505  yarn create react-app testapp
  506  cd testapp/
  507  yarn add getstorybook
  508  ./node_modules/.bin/getstorybook 
  509 cd ../storybook
  510 jscodeshift -t ./lib/codemod/src/transforms/update-organisation-name.js --ignore-pattern "node_modules|dist"

@ndelangen ndelangen force-pushed the am-fix-codemod-docs branch from bfce2cb to c6bddac Compare May 27, 2017 11:09
Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

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

GREAT!!!

@shilman shilman merged commit 85d22ee into master May 27, 2017
@shilman shilman deleted the am-fix-codemod-docs branch May 27, 2017 11:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation maintenance User-facing maintenance tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants