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

migrate tool ignores parser #8091

Closed
jdelStrother opened this issue Sep 16, 2019 · 8 comments
Closed

migrate tool ignores parser #8091

jdelStrother opened this issue Sep 16, 2019 · 8 comments

Comments

@jdelStrother
Copy link

jdelStrother commented Sep 16, 2019

Describe the bug
When running sb migrate with a parser argument (eg sb migrate --parser=flow storiesof-to-csf --glob 'components/stories/*.js'), the parser is ignored.

To Reproduce
Steps to reproduce the behavior:

  1. Create a stories file with flowtype annotations
  2. Run sb migrate --parser=flow storiesof-to-csf --glob 'components/stories/*.js'

Expected behavior
The migrate tool converts my stories to CSF format

Screenshots
Failure due to babel parser not being able to parse flow types:

 ERR components/stories/midrollToolbar.js Transformation error
SyntaxError: Unexpected token, expected ";" (7:11)
    at _class.raise (/Users/jon/.config/yarn/global/node_modules/jscodeshift/node_modules/babylon/lib/index.js:776:15)
    at _class.unexpected (/Users/jon/.config/yarn/global/node_modules/jscodeshift/node_modules/babylon/lib/index.js:2079:16)
    at _class.semicolon (/Users/jon/.config/yarn/global/node_modules/jscodeshift/node_modules/babylon/lib/index.js:2063:40)
    at _class.parseVarStatement (/Users/jon/.config/yarn/global/node_modules/jscodeshift/node_modules/babylon/lib/index.js:4413:10)
    at _class.parseStatementContent (/Users/jon/.config/yarn/global/node_modules/jscodeshift/node_modules/babylon/lib/index.js:4012:21)
    at _class.parseStatement (/Users/jon/.config/yarn/global/node_modules/jscodeshift/node_modules/babylon/lib/index.js:3962:17)
    at _class.parseBlockOrModuleBlockBody (/Users/jon/.config/yarn/global/node_modules/jscodeshift/node_modules/babylon/lib/index.js:4513:23)
    at _class.parseBlockBody (/Users/jon/.config/yarn/global/node_modules/jscodeshift/node_modules/babylon/lib/index.js:4500:10)
    at _class.parseBlockBody (/Users/jon/.config/yarn/global/node_modules/jscodeshift/node_modules/babylon/lib/index.js:5462:44)
    at _class.parseTopLevel (/Users/jon/.config/yarn/global/node_modules/jscodeshift/node_modules/babylon/lib/index.js:3938:10)

Additional context

The parser option is passed here -

migrate(migration, { configDir, glob, dryRun, list, rename, parser, logger }).catch(err => {
- and then ignored here -
export async function migrate(migration, { configDir, glob, dryRun, list, rename, logger }) {
.

I tried to naively fix it by passing the parserOption through to runCodeMod, but it dies at the prettier stage:

 ERR components/stories/midrollToolbar.js Transformation error (Cannot read property 'process' of undefined)
TypeError: Cannot read property 'process' of undefined
    at buffer (/Users/jon/Developer/web/webpack/node_modules/prettier/parser-flow.js:1:42536)
    at mGt (/Users/jon/Developer/web/webpack/node_modules/prettier/parser-flow.js:1:1429771)
    at e (/Users/jon/Developer/web/webpack/node_modules/prettier/parser-flow.js:1:393)
    at default (/Users/jon/Developer/web/webpack/node_modules/prettier/parser-flow.js:1:19587)
    at /Users/jon/Developer/web/webpack/node_modules/prettier/parser-flow.js:1:16
    at Object.prettierPlugins (/Users/jon/Developer/web/webpack/node_modules/prettier/parser-flow.js:1:139)
    at Module._compile (internal/modules/cjs/loader.js:816:30)
    at Module._compile (/Users/jon/Developer/web/webpack/node_modules/pirates/lib/index.js:99:24)
    at Module._extensions..js (internal/modules/cjs/loader.js:827:10)
    at Object.newLoader [as .js] (/Users/jon/Developer/web/webpack/node_modules/pirates/lib/index.js:104:7)
@shilman
Copy link
Member

shilman commented Sep 16, 2019

@jdelStrother I'm a little confused. Is the option getting ignored, or getting recognized and then failing? It looks to me like it's getting passed properly in the code--I haven't set up a repro yet. Thanks!

@jdelStrother
Copy link
Author

@shilman it's getting ignored. cli/bin/generate.js does pass the migrate parameter through to cli/lib/migrate.js, but that then just ignores it

export async function migrate(migration, { configDir, glob, dryRun, list, rename, logger }) {

@jdelStrother
Copy link
Author

Something like this would fix the parser option getting ignored -

import { listCodemods, runCodemod } from '@storybook/codemod';

- export async function migrate(migration, { configDir, glob, dryRun, list, rename, logger }) {
+ export async function migrate(migration, { configDir, glob, dryRun, list, rename, logger, parser }) {

  if (list) {
    listCodemods().forEach(key => logger.log(key));
  } else if (migration) {
-    await runCodemod(migration, { configDir, glob, dryRun, logger, rename });
+    await runCodemod(migration, { configDir, glob, dryRun, logger, rename, parser });
  } else {
    throw new Error('Migrate: please specify a migration name or --list');
  }
}

but that then fails in new and interesting ways inside the codemod - eg

 ERR components/stories/midrollToolbar.js Transformation error (Cannot read property 'process' of undefined)
TypeError: Cannot read property 'process' of undefined
    at buffer (/Users/jon/Developer/web/webpack/node_modules/prettier/parser-flow.js:1:42536)
    at mGt (/Users/jon/Developer/web/webpack/node_modules/prettier/parser-flow.js:1:1429771)
    at e (/Users/jon/Developer/web/webpack/node_modules/prettier/parser-flow.js:1:393)
    at default (/Users/jon/Developer/web/webpack/node_modules/prettier/parser-flow.js:1:19587)
    at /Users/jon/Developer/web/webpack/node_modules/prettier/parser-flow.js:1:16
    at Object.prettierPlugins (/Users/jon/Developer/web/webpack/node_modules/prettier/parser-flow.js:1:139)
    at Module._compile (internal/modules/cjs/loader.js:816:30)
    at Module._compile (/Users/jon/Developer/web/webpack/node_modules/pirates/lib/index.js:99:24)
    at Module._extensions..js (internal/modules/cjs/loader.js:827:10)
    at Object.newLoader [as .js] (/Users/jon/Developer/web/webpack/node_modules/pirates/lib/index.js:104:7)

@mbylstra
Copy link

There's another way you can run the storiesof-to-csf transform as described in https://www.npmjs.com/package/@storybook/codemod.

Eg: ./node_modules/.bin/jscodeshift --parser=tsx -t ./node_modules/@storybook/codemod/dist/transforms/storiesof-to-csf.js . --ignore-pattern "node_modules|dist"

This should bypass the problem of the --parser=flow argument not being passed to jscodeshift. There's still potentially further problems. I'm using this technique this to parse xxxx.stories.tsx files. The parser is working, but the transform itself appears to not work on the Typescript AST. It's removing the the storiesOf import statement, so it's doing something, but it's not doing anything more than that.

@shilman
Copy link
Member

shilman commented Jan 29, 2020

@mbylstra somebody told me that it’s actually prettier that’s falling on TSX and not the codemod itself. And that if you comment out the prettier invocation in the package dist it will succeed. I’m on vacation this week so i haven’t had time to investigate. But if it’s easy for you to verify that, it would help give an extra data point for a proper fix

@mbylstra
Copy link

Yes, thanks - I also found that. But then I ran into further problems with the transform when it's run in tsx mode: I opened an issue about this: #9669

@stale
Copy link

stale bot commented Dec 26, 2020

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

@stale stale bot added the inactive label Dec 26, 2020
@shilman
Copy link
Member

shilman commented Dec 28, 2020

Fixed in #12453 and released in 6.0. Closing!

@shilman shilman closed this as completed Dec 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants