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

Add dotenv-webpack to work around storybookjs/storybook#14403 #821

Merged
merged 1 commit into from
Jun 9, 2021

Conversation

leonm1
Copy link
Member

@leonm1 leonm1 commented Jun 9, 2021

storybookjs/storybook#14497 and storybookjs/storybook#14403

tl;dr: Happo stopped working because npm run build-storybook was failing after #818 upgraded webpack to a new minor version and threw us into the weird implicit hoisting mess described below.

Theoretically this is a workaround and can be removed after storybookjs/storybook#14403 is resolved, but there is no sign of when that'll happen

Deep technical reasons from the linked issue for why this fails:

If npm hoists dotenv-webpack 6.0.4 to the node_modules root, everything works fine. However, if npm hoists dotenv-webpack 1.8, and the user has a .env file present, start-storybook fails to run.

Unless the user is already pinning dotenv-webpack, the version that gets hoisted depends on how many dependencies are using 1.8 vs 6.0.4. (For example, in the repro steps below, if you remove @storybook/addon-essentials, npm hoists 6.0.4 and things work normally.) This makes for some extremely surprising and hard-to-pin-down behavior.

storybookjs/storybook#14497 and storybookjs/storybook#14403

tl;dr: Happo stopped working after #818 was merged, it turns out it is because
`npm run build-storybook` crashed due to the above issue(s).

Theoretically this is a workaround and can be removed after storybookjs/storybook#14403 is resolved, but there is no sign of when that'll happen

Deep technical reasons from the linked issue for why this fails:

> If npm hoists dotenv-webpack 6.0.4 to the node_modules root, everything works fine. However, if npm hoists dotenv-webpack 1.8, and the user has a .env file present, start-storybook fails to run.
>
> Unless the user is already pinning dotenv-webpack, the version that gets hoisted depends on how many dependencies are using 1.8 vs 6.0.4. (For example, in the repro steps below, if you remove @storybook/addon-essentials, npm hoists 6.0.4 and things work normally.) This makes for some extremely surprising and hard-to-pin-down behavior.
@leonm1 leonm1 enabled auto-merge (squash) June 9, 2021 21:02
@codeclimate
Copy link

codeclimate bot commented Jun 9, 2021

Code Climate has analyzed commit 836b4dc and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (0% is the threshold).

This pull request will bring the total coverage in the repository to 63.4% (0.0% change).

View more on Code Climate.

@leonm1
Copy link
Member Author

leonm1 commented Jun 9, 2021

As you can see, the CI / Happo Screnshot Diffs (pull_request) check now completes successfully

@leonm1 leonm1 disabled auto-merge June 9, 2021 21:06
@leonm1 leonm1 merged commit 704e098 into main Jun 9, 2021
@leonm1 leonm1 deleted the fix/happo branch June 9, 2021 21:06
@SamLee514 SamLee514 assigned SamLee514 and unassigned SamLee514 Jun 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants