Skip to content
This repository has been archived by the owner on Mar 25, 2023. It is now read-only.

Commit

Permalink
Fix bug with webpack5, storybook 6.4.0-alpha, and react-docgen-typesc…
Browse files Browse the repository at this point in the history
…ript-plugin

- tl;dr : Add a resolutions field. Note that this field needs to
  be in the root workspace to have any effect, and so therefore
  must be repeated in the root package.json of both this repo and
  its parent monorepo.
- Add a longer explanation of what happened in the .ci/chromatic.sh
  script, since package.json doesn't allow comments, and since
  the next time this breaks, it's gonna be in that file
- Bump webpack to absolute cutting edge bleeding nightly super early bird
- In storybook config, explicitly configure react-docgen-typescript-plugin,
  although I have no reason to think it made a difference
- Allow specifying tsconfig file via env var TSCONFIG_FILE, default to tsconfig.json

CU-qthhx6

ref storybookjs/storybook#15336
ref https://github.com/hipstersmoothie/react-docgen-typescript-plugin/blob/31ce4e84008a45269b8524157ad51bf9d191acac/package.json#L81
ref https://gist.github.com/shilman/8856ea1786dcd247139b47b270912324
  • Loading branch information
milesrichardson committed Aug 23, 2021
1 parent 4389fe1 commit 9c90f6a
Show file tree
Hide file tree
Showing 6 changed files with 111 additions and 219 deletions.
77 changes: 77 additions & 0 deletions .ci/chromatic.sh
Expand Up @@ -47,3 +47,80 @@ yarn dlx chromatic \
&& exit 0

exit 1
# the end. remainder of file is for debugging and notes

# When debugging, you probably just want to run a build, so uncomment this:
# yarn run storybook-build \
# --output-dir out-storybook \
# && exit 0

# NOTE
# TEMPORARY WORKAROUND for upstream bugs amidst the ecosystem migration to webpack 5
# [Commenting here since this is the file that could have a CI failure due to this.]
#
# The relevant code is actually in `splitgraph.com/package.json`, but I can't
# leave comments in JS files, and this deserves an explanation...
#
# "@storybook/react/webpack": "5.51.1"
#
# We are telling @storybook/react to resolve webpack to 5.51.1, which means it
# will _provide_ webpack@5.51.1 as a peerDependency to the buggy package
# below it, `react-docgen-typescript-plugin`, which we want to force to
# accept webpack 5 instead of webpack 4 while both are within its range of >= 4.
#
# -- PROBLEM:
# Problem is `react-docgen-typescript-plugin` peerDependency webpack @ >= 4
# We're using webpack 5 for storybook compilation, but it's experimental, and
# Storybook still uses webpack 4 for its "manager interface." So we need to
# keep a copy of both webpack 4 and webpack 5.
#
# webpack 5 is _not_ hoisted (it's in docs)
# webpack 4 _is_ hoisted (it goes to root)
#
# There is a bug (IMO) in `react-docgen-typescript-plugin`, where it tries to
# import directly from the resolved location of webpack 4, even though it's
# attempting to import files that only exist in webpack 5. The maintainer is
# not going to drop webpack 4 until the next major release.
#
# n.b: This problem will only reproduce when `splitgraph/splitgraph.com` repo
# is running in "isolation", i.e. during CI or by a developer outside the monorepo.
#
# To reproduce locally, you can use ACT or just clone the repo to a temp dir.
# For details on using ACT, see README.md
#
# -- FIX:
# yarn resolutions to the rescue! We want to force `react-docgen-typescript-plugin`
# to resolve webpack 4 to webpack 5, but _without_ overriding resolutions of other
# packages that depend on webpack 4 (like this alleged manager code).
#
# But since it's a peer dependency, the package has no say in what version satisfies
# its range of >= 4. So setting a resolution for that package wouldn't do anything.
#
# Instead, we want to tell the _parent_ of the buggy package to _provide_ it with
# webpack 5 as a peerDependency to satisfy the range of `webpack >= 4`.
#
# In this case, the parent is `@storybook/react`, and it's _providing_ `webpack`
# so we need to use `@storybook/react/webpack` as the key in the `resolutions` field.

# Note: This will likely also change the version of webpack provided to any other
# direct dependencies of `@storybook/react` that have a webpack peer dependency
# of any version. This hasn't caused problems yet.
#
#[More targeted approach would be to `yarn patch` that buggy
# repository and change its peerDependencies to 5. ]
#
# DEBUGGING / INVESTIGATING IF HACK IS STILL NECESSARY
#
# Eventually this bug will be fixed and this hack will be unnecessary.
# Here are some helpful commands for investigating (but first, reproduce the error)
#
# # find the relevant peer-requirement. Need 5 digit pHash, e.g. p387a7
# yarn explain peer-requirements | grep webpack | grep docgen
#
# # explain that specific ID
# yarn explain peer-requirements p387a7
#
# # set the resolution for webpack to pin at 5.51.1
# # (we will want 5.51.1 to match whatever version of webpack is installed)
# yarn set resolution @storybook/react-docgen-typescript-plugin@*/webpack 5.51.1
#
2 changes: 1 addition & 1 deletion .github/workflows/build_and_deploy_preview.yml
Expand Up @@ -54,7 +54,7 @@ jobs:
# Typecheck
- uses: actions/cache@v1
with:
path: ${github.workspace}
path: ${github.workspace}/dist
key: typecache
- name: Typecheck
id: typecheck
Expand Down
23 changes: 23 additions & 0 deletions docs/.storybook/main.js
@@ -1,7 +1,30 @@
const TSCONFIG_PATH = require("path").join(
__dirname,
"..",
process.env.TSCONFIG_PATH ?? `tsconfig.json`
);

console.log("TSCONFIG_PATH:", TSCONFIG_PATH);

module.exports = {
core: {
builder: "webpack5",
},
// https://storybook.js.org/docs/react/configure/typescript
typescript: {
check: false,
checkOptions: {},
reactDocgen: "react-docgen-typescript",

// https://github.com/hipstersmoothie/react-docgen-typescript-plugin
// https://github.com/styleguidist/react-docgen-typescript#parseroptions
// https://github.com/hipstersmoothie/react-docgen-typescript-plugin#debugging
reactDocgenTypescriptOptions: {
shouldExtractLiteralValuesFromEnum: true,
propFilter: (prop) =>
prop.parent ? !/node_modules/.test(prop.parent.fileName) : true,
},
},
reactOptions: {
fastRefresh: true,
},
Expand Down
2 changes: 1 addition & 1 deletion docs/package.json
Expand Up @@ -76,7 +76,7 @@
"@types/styled-system": "5.1.10",
"babel-loader": "8.2.2",
"now": "18.0.0",
"webpack": "5.28.0"
"webpack": "5.51.1"
},
"peerDependencies": {
"@emotion/react": "*",
Expand Down
3 changes: 3 additions & 0 deletions package.json
Expand Up @@ -16,6 +16,9 @@
"templaters",
"lib"
],
"resolutions": {
"@storybook/react/webpack": "5.51.1"
},
"scripts": {
"typecheck-and-build": "yarn run typecheck && yarn run build",
"build": "yarn workspace @splitgraph/docs run build",
Expand Down

0 comments on commit 9c90f6a

Please sign in to comment.