-
-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
Core: Fix webpack stats #14198
Core: Fix webpack stats #14198
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CI is failing due to something stats related. can you take a look @tmeasday ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aha, LGTM 🙏
@@ -8,7 +8,7 @@ | |||
"do-storyshots-puppeteer": "../../node_modules/.bin/jest --projects=./storyshots-puppeteer", | |||
"generate-addon-jest-testresults": "jest --config=tests/addon-jest.config.json --json --outputFile=stories/addon-jest.testresults.json", | |||
"graphql": "node ./graphql-server/index.js", | |||
"packtracker": "yarn storybook --smoke-test --webpack-stats-json --quiet && cross-env PT_PROJECT_TOKEN=1af1d41b-d737-41d4-ac00-53c8f3913b53 packtracker-upload --stats=./node_modules/.cache/storybook/public/manager-stats.json", | |||
"packtracker": "yarn storybook --smoke-test --webpack-stats-json /tmp --quiet && cross-env PT_PROJECT_TOKEN=1af1d41b-d737-41d4-ac00-53c8f3913b53 packtracker-upload --stats=/tmp/manager-stats.json", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"packtracker": "yarn storybook --smoke-test --webpack-stats-json /tmp --quiet && cross-env PT_PROJECT_TOKEN=1af1d41b-d737-41d4-ac00-53c8f3913b53 packtracker-upload --stats=/tmp/manager-stats.json", | |
"packtracker": "yarn storybook --smoke-test --webpack-stats-json=/tmp --quiet && cross-env PT_PROJECT_TOKEN=1af1d41b-d737-41d4-ac00-53c8f3913b53 packtracker-upload --stats=/tmp/manager-stats.json", |
export const makeStatsFromError: (err: string) => Stats = (err) => | ||
({ | ||
hasErrors: () => true, | ||
hasWarnings: () => false, | ||
toJson: () => ({ warnings: [] as any[], errors: [err] }), | ||
} as any); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this something we want to export?
In the future we'll want to map the builders internal stats object to something common for all builders.
Right now we're leaking webpack details from our 2 builders. This is a known issue, but we should be really careful to not make that situation worse.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm thinking something like SyntacticEvent in React, where the underlaying raw event is still available, but there's a layer on top to smooth out the browser differences. (except we're smoothing out the builder differences)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I think that's the plan for 6.3 (or whenever we add our next builder).
We probably shouldn't be exporting this function however, that was an oversight.
'Write Webpack Stats JSON to disk', | ||
resolvePathInStorybookCache(`public/`) | ||
) | ||
.option('--webpack-stats-json [directory]', 'Write Webpack Stats JSON to disk') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You forgot the <directory>
'Write Webpack Stats JSON to disk', | ||
resolvePathInStorybookCache(`public/`) | ||
) | ||
.option('--webpack-stats-json [directory]', 'Write Webpack Stats JSON to disk') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You forgot <directory>
Issue: #14139
What I did
Forgot to push this final commit on other PR (!)