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.

Sign up for GitHub

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

Jump to bottom

storysource: Add default parser option. Support prettier v1.13.0 #3660

Merged
merged 10 commits into from Jun 3, 2018
+76 −33
Merged

storysource: Add default parser option. Support prettier v1.13.0 #3660

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
1903555
storysource: Add default parser option. Support prettier v0.13.0 ( #3…
isoppp May 28, 2018
8ec5a40
Revert "storysource: Add default parser option. Support prettier v0.1…
isoppp May 30, 2018
86a4cb1
storysource: pass file path to prettier config / support to parser ch…
isoppp May 30, 2018
35cd5cb
revert yarn.lock #3657
isoppp May 30, 2018
b166433
Merge remote-tracking branch 'origin/master' into issue-3657-storysou…
isoppp May 30, 2018
3610c72
storybook: unlock prettier minor version
isoppp May 30, 2018
7f2864b
storysource: update prettier version to 1.13.3 #3657
isoppp May 31, 2018
a450f4e
storysource: organized the options to pass to Prettier #3657
isoppp May 31, 2018
05e419c
Revert "storysource: organized the options to pass to Prettier #3657"
isoppp Jun 1, 2018
6ac4983
storysource: Modify to make parser or filepath always specified #3657
isoppp Jun 1, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion addons/storysource/package.json
View file
Open in desktop
Expand Up @@ -25,7 +25,7 @@
"babel-runtime": "^6.26.0",
"estraverse": "^4.2.0",
"loader-utils": "^1.1.0",
"prettier": "~1.12.1",
"prettier": "^1.13.3",
"prop-types": "^15.6.1",
"react-syntax-highlighter": "^7.0.4"
},
Expand Down
24 changes: 18 additions & 6 deletions addons/storysource/src/loader/generate-helpers.js
View file
Open in desktop
Expand Up @@ -27,14 +27,26 @@ function generateSourceWithoutUglyComments(source, { comments, uglyCommentsRegex
return parts.join('');
}

function prettifyCode(source, { prettierConfig, parser }) {
function prettifyCode(source, { prettierConfig, parser, filepath }) {
let config = prettierConfig;

if (!config.parser && parser && parser !== 'javascript') {
config = {
...prettierConfig,
parser,
};
if (!config.parser) {
if (parser) {
config = {
...prettierConfig,
parser: parser === 'javascript' ? 'babylon' : parser,
Copy link
Member

@Hypnosphi Hypnosphi May 30, 2018 •
edited

Choose a reason for hiding this comment

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

Not needed anymore, filepath is enough

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought the same.
But in this test , if the extension is txt and the contents are something else, prettier does not support to txt file, so it gets an error.

error:

No parser could be inferred for file: (LOCAL_PATH)/addons/storysource/src/loader/__mocks__/inject-decorator.stories.txt

Should i change the test?
I think that probably this is a rare case, so I think that do not have to deal with it unless need it.

Copy link
Member

Choose a reason for hiding this comment

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

After giving it a second thought, we probably should always pass parser to prettier when user specifies it explicitly, which is exactly what we do now. So let's keep it as is

Copy link
Member

@Hypnosphi Hypnosphi May 31, 2018 •
edited

Choose a reason for hiding this comment

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

But we probably should put else if (filepath) inside if (!config.parser), to cover cases when parser is not passed

Copy link
Member

Choose a reason for hiding this comment

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

Or maybe we should just pass babylon when parser isn't set explicitly, because this is what we use by default ourselves. In this case, filepath will be never needed

Copy link
Contributor Author

@isoppp isoppp May 31, 2018 •
edited

Choose a reason for hiding this comment

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

Thank you for review. I tried fixing it.

And, i read Prettier code a little.
Prettier gives priority to parser over filepath.
If both are empty we gave babylon as Prettier will output errors in the future.

};
} else if (filepath) {
config = {
...prettierConfig,
filepath,
};
} else {
config = {
...prettierConfig,
parser: 'babylon',
};
}
}

return prettier.format(source, config);
Expand Down
2 changes: 1 addition & 1 deletion addons/storysource/src/loader/index.js
View file
Open in desktop
Expand Up @@ -5,7 +5,7 @@ const ADD_DECORATOR_STATEMENT = '.addDecorator(withStorySource(__STORY__, __ADDS

function transform(source) {
const options = getOptions(this) || {};
const result = injectDecorator(source, ADD_DECORATOR_STATEMENT, options);
const result = injectDecorator(source, ADD_DECORATOR_STATEMENT, this.resourcePath, options);

if (!result.changed) {
return source;
Expand Down
7 changes: 4 additions & 3 deletions addons/storysource/src/loader/inject-decorator.js
View file
Open in desktop
Expand Up @@ -6,16 +6,17 @@ import {
generateAddsMap,
} from './generate-helpers';

function extendOptions(source, comments, options) {
function extendOptions(source, comments, filepath, options) {
return {
...defaultOptions,
...options,
source,
comments,
filepath,
};
}

function inject(source, decorator, options = {}) {
function inject(source, decorator, filepath, options = {}) {
const { changed, source: newSource, comments } = generateSourceWithDecorators(
source,
decorator,
Expand All @@ -30,7 +31,7 @@ function inject(source, decorator, options = {}) {
};
}

const storySource = generateStorySource(extendOptions(source, comments, options));
const storySource = generateStorySource(extendOptions(source, comments, filepath, options));
const addsMap = generateAddsMap(storySource, options.parser);

return {
Expand Down
62 changes: 46 additions & 16 deletions addons/storysource/src/loader/inject-decorator.test.js
View file
Open in desktop
@@ -1,12 +1,19 @@
import fs from 'fs';
import path from 'path';
import injectDecorator from './inject-decorator';

const ADD_DECORATOR_STATEMENT = '.addDecorator(withStorySource(__STORY__, __ADDS_MAP__))';

describe('inject-decorator', () => {
describe('positive', () => {
const source = fs.readFileSync('./__mocks__/inject-decorator.stories.txt', 'utf-8');
const result = injectDecorator(source, ADD_DECORATOR_STATEMENT);
const mockFilePath = './__mocks__/inject-decorator.stories.txt';
const source = fs.readFileSync(mockFilePath, 'utf-8');
const result = injectDecorator(
source,
ADD_DECORATOR_STATEMENT,
path.resolve(__dirname, mockFilePath),
{ parser: 'javascript' }
);

it('returns "changed" flag', () => {
expect(result.changed).toBeTruthy();
Expand All @@ -22,8 +29,14 @@ describe('inject-decorator', () => {
});

describe('positive - angular', () => {
const source = fs.readFileSync('./__mocks__/inject-decorator.angular-stories.txt', 'utf-8');
const result = injectDecorator(source, ADD_DECORATOR_STATEMENT, { parser: 'typescript' });
const mockFilePath = './__mocks__/inject-decorator.angular-stories.txt';
const source = fs.readFileSync(mockFilePath, 'utf-8');
const result = injectDecorator(
source,
ADD_DECORATOR_STATEMENT,
path.resolve(__dirname, mockFilePath),
{ parser: 'typescript' }
);

it('returns "changed" flag', () => {
expect(result.changed).toBeTruthy();
Expand All @@ -39,8 +52,14 @@ describe('inject-decorator', () => {
});

describe('positive - ts', () => {
const source = fs.readFileSync('./__mocks__/inject-decorator.ts.txt', 'utf-8');
const result = injectDecorator(source, ADD_DECORATOR_STATEMENT, { parser: 'typescript' });
const mockFilePath = './__mocks__/inject-decorator.ts.txt';
const source = fs.readFileSync(mockFilePath, 'utf-8');
const result = injectDecorator(
source,
ADD_DECORATOR_STATEMENT,
path.resolve(__dirname, mockFilePath),
{ parser: 'typescript' }
);

it('returns "changed" flag', () => {
expect(result.changed).toBeTruthy();
Expand All @@ -56,33 +75,44 @@ describe('inject-decorator', () => {
});

describe('stories with ugly comments', () => {
const source = fs.readFileSync(
'./__mocks__/inject-decorator.ugly-comments-stories.txt',
'utf-8'
const mockFilePath = './__mocks__/inject-decorator.ugly-comments-stories.txt';
const source = fs.readFileSync(mockFilePath, 'utf-8');
const result = injectDecorator(
source,
ADD_DECORATOR_STATEMENT,
path.resolve(__dirname, mockFilePath),
{ parser: 'javascript' }
);
const result = injectDecorator(source, ADD_DECORATOR_STATEMENT);

it('should delete ugly comments from the generated story source', () => {
expect(result.storySource).toMatchSnapshot();
});
});

describe('stories with ugly comments in ts', () => {
const source = fs.readFileSync(
'./__mocks__/inject-decorator.ts.ugly-comments-stories.txt',
'utf-8'
const mockFilePath = './__mocks__/inject-decorator.ts.ugly-comments-stories.txt';
const source = fs.readFileSync(mockFilePath, 'utf-8');
const result = injectDecorator(
source,
ADD_DECORATOR_STATEMENT,
path.resolve(__dirname, mockFilePath),
{ parser: 'typescript' }
);
const result = injectDecorator(source, ADD_DECORATOR_STATEMENT, { parser: 'typescript' });

it('should delete ugly comments from the generated story source', () => {
expect(result.storySource).toMatchSnapshot();
});
});

it('will not change the source when there are no "storiesOf" functions', () => {
const source = fs.readFileSync('./__mocks__/inject-decorator.no-stories.txt', 'utf-8');
const mockFilePath = './__mocks__/inject-decorator.no-stories.txt';
const source = fs.readFileSync(mockFilePath, 'utf-8');

const result = injectDecorator(source, ADD_DECORATOR_STATEMENT);
const result = injectDecorator(
source,
ADD_DECORATOR_STATEMENT,
path.resolve(__dirname, mockFilePath)
);

expect(result.changed).toBeFalsy();
expect(result.addsMap).toEqual({});
Expand Down
2 changes: 1 addition & 1 deletion addons/storysource/src/loader/parsers/parser-js.js
View file
Open in desktop
@@ -1,7 +1,7 @@
import parseJs from 'prettier/parser-babylon';

function parse(source) {
return parseJs(source);
return parseJs.parsers.babylon.parse(source);
}

export default {
Expand Down
2 changes: 1 addition & 1 deletion addons/storysource/src/loader/parsers/parser-ts.js
View file
Open in desktop
@@ -1,7 +1,7 @@
import parseTs from 'prettier/parser-typescript';

function parse(source) {
return parseTs(source);
return parseTs.parsers.typescript.parse(source);
}

export default {
Expand Down
8 changes: 4 additions & 4 deletions yarn.lock
View file
Open in desktop
Expand Up @@ -13407,14 +13407,14 @@ prettier@^1.13.0:
version "1.13.2"
resolved "https://registry.yarnpkg.com/prettier/-/prettier-1.13.2.tgz#412b87bc561cb11074d2877a33a38f78c2303cda"

prettier@^1.13.3:
version "1.13.3"
resolved "https://registry.yarnpkg.com/prettier/-/prettier-1.13.3.tgz#e74c09a7df6519d472ca6febaa37cf7addb48a20"

prettier@^1.7.0:
version "1.10.2"
resolved "https://registry.yarnpkg.com/prettier/-/prettier-1.10.2.tgz#1af8356d1842276a99a5b5529c82dd9e9ad3cc93"

prettier@~1.12.1:
version "1.12.1"
resolved "https://registry.yarnpkg.com/prettier/-/prettier-1.12.1.tgz#c1ad20e803e7749faf905a409d2367e06bbe7325"

pretty-bytes@^4.0.2:
version "4.0.2"
resolved "https://registry.yarnpkg.com/pretty-bytes/-/pretty-bytes-4.0.2.tgz#b2bf82e7350d65c6c33aa95aaa5a4f6327f61cd9"
Expand Down