Skip to content
This repository was archived by the owner on Apr 26, 2024. It is now read-only.

Storybook won't start without file-loader dependency installed#56

Merged
mmv08 merged 1 commit intodevelopmentfrom
fix-storybook-build
Aug 13, 2020
Merged

Storybook won't start without file-loader dependency installed#56
mmv08 merged 1 commit intodevelopmentfrom
fix-storybook-build

Conversation

@alfetopito
Copy link
Contributor

Trying to run yarn storybook from a fresh development branch install fails:

screenshot_2020-08-12_10-41-45

After adding missing dependency it works

@alfetopito alfetopito self-assigned this Aug 12, 2020
@alfetopito alfetopito requested a review from mmv08 August 12, 2020 17:42
@github-actions
Copy link

ESLint Summary View Full Report

Annotations are provided inline on the Files Changed tab. You can also see all annotations that were generated on the annotations page.

Type Occurrences Fixable
Errors 0 0
Warnings 0 0
Ignored 0 N/A
  • Result: ✅ success
  • Annotations: 0 total

Report generated by eslint-plus-action

@ghost
Copy link

ghost commented Aug 12, 2020

Travis automatic deployment:
https://pr56--safereactcomponents.review.gnosisdev.com

@fernandomg
Copy link
Contributor

fernandomg commented Aug 12, 2020

@alfetopito, thanks!

I'm not sure if adding a dependency of a dependency is the solution, tho.

Funny thing, I fixed the same issue with a different approach: by downgrading webpack to the previous version. https://github.com/gnosis/safe-react-components/pull/55/files#diff-b9cfc7f2cdf78a7f4b91a753d10865a2R69

... and now I see there's a 0.44.1 that may fix this seamlessly.

Edit: no, it didn't worked
image

@alfetopito
Copy link
Contributor Author

@alfetopito, thanks!

I'm not sure if adding a dependency of a dependency is the solution, tho.

I'm not sure it qualifies as a dependency of a dependency.
It's used directly on https://github.com/gnosis/safe-react-components/blob/b83b411442aefba7b8335baa247bb9dbc872a9ec/webpack.config.js#L22 as well as the storybook webpack config https://github.com/gnosis/safe-react-components/blob/b83b411442aefba7b8335baa247bb9dbc872a9ec/.storybook/webpack.config.js#L18

Funny thing, I fixed the same issue with a different approach: by downgrading webpack to the previous version. https://github.com/gnosis/safe-react-components/pull/55/files#diff-b9cfc7f2cdf78a7f4b91a753d10865a2R69

... and now I see there's a 0.44.1 that may fix this seamlessly.

Edit: no, it didn't worked
image

Now if you say this should come bundled with Webpack, then I agree.
But is it?

For example, dex-react also sets it as a dev dependency https://github.com/gnosis/dex-react/search?q=file-loader&unscoped_q=file-loader

@mmv08
Copy link
Contributor

mmv08 commented Aug 12, 2020

If you inspect yarn.lock it seems that we have two file-loader installed, one is a dependency of storybook core and one is the one we explicitly add as a dep. Looks like it was working previously because of that storybook dependency. I think it's good to include it in our dependencies list if we depend on it.

@alfetopito
Copy link
Contributor Author

I'm not familiar with your team's dev practices, I can leave it up to you to merge it.

Otherwise I can do it if you don't mind and tell me which I should pick:
screenshot_2020-08-12_13-19-12

That is, if one approval is enough.

@mmv08
Copy link
Contributor

mmv08 commented Aug 13, 2020

@alfetopito usually we squash to dev and a regular merge to master

@mmv08 mmv08 merged commit b459cc9 into development Aug 13, 2020
@mmv08 mmv08 deleted the fix-storybook-build branch August 13, 2020 07:44
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants