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

Addon Storysource typescript support #3253

Merged
merged 4 commits into from
Mar 21, 2018

Conversation

igor-dv
Copy link
Member

@igor-dv igor-dv commented Mar 21, 2018

Issue: acorn parser doesn't support typescript out of the box. Until now all the angular stories were parsed successfully because there was no typing in them. But if you want to add a component example into the story (I think it is useful because the user will see a real example and not only a story) the parsing breaks.

What I did

I've removed the use of acorn from the loader and used prettier/parser-* instead (since the addon is already dependant on prettier).

In this case for a regular javascript (which is the default), we will use prettier/parser-babylon (according to the docs babylon is based on acorn)
And for TypeScript we will use prettier/parser-typescript.
In order to make it work with typescript, I've added a parser option to the loader.

{
  loader: require.resolve('@storybook/addon-storysource/loader'),
  options: {
    parser: 'typescript',
  },
},

Copy link
Member

@Hypnosphi Hypnosphi left a comment

Choose a reason for hiding this comment

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

Cool. AFAIU it will allow parsing Flow as well (at least prettier itself does it with no problem)

@igor-dv
Copy link
Member Author

igor-dv commented Mar 21, 2018

AFAIU it will allow parsing Flow as well

Yeah. they have a parser-flow as well =)

https://github.com/prettier/prettier/tree/master/src/language-js

@codecov
Copy link

codecov bot commented Mar 21, 2018

Codecov Report

Merging #3253 into master will decrease coverage by <.01%.
The diff coverage is 61.76%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3253      +/-   ##
==========================================
- Coverage   35.86%   35.85%   -0.01%     
==========================================
  Files         440      444       +4     
  Lines        9682     9731      +49     
  Branches      902      927      +25     
==========================================
+ Hits         3472     3489      +17     
+ Misses       5647     5627      -20     
- Partials      563      615      +52
Impacted Files Coverage Δ
addons/storysource/src/loader/parsers/parser-ts.js 14.28% <25%> (ø)
addons/storysource/src/loader/parsers/parser-js.js 14.28% <25%> (ø)
addons/storysource/src/loader/parsers/index.js 44.44% <57.14%> (ø)
addons/storysource/src/loader/generate-helpers.js 44.44% <64.7%> (-3.66%) ⬇️
addons/storysource/src/loader/inject-decorator.js 28% <66.66%> (-2.44%) ⬇️
addons/storysource/src/loader/parse-helpers.js 46.75% <69.23%> (-1.59%) ⬇️
addons/storysource/src/loader/traverse-helpers.js 60.86% <70%> (ø)
app/vue/src/server/utils.js 0% <0%> (-100%) ⬇️
... and 61 more

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 6066967...eae6cba. Read the comment docs.

@Hypnosphi Hypnosphi merged commit 1d104cd into master Mar 21, 2018
@Hypnosphi Hypnosphi deleted the addon-storysource-typescript-support branch March 21, 2018 18:02
@Hypnosphi
Copy link
Member

Released as 4.0.0-alpha.0

@Hypnosphi
Copy link
Member

Hypnosphi commented May 29, 2018

Looks like Prettier doesn't consider those parsers a public API: they changed their exports in a minor release

Can we maybe use @babel/parser and typescript-eslint-parser directly?

@igor-dv
Copy link
Member Author

igor-dv commented May 30, 2018

Probably we should, they might break it again. Maybe to worth porting the "generic parser" as a separate repo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants