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

React: Switch react-docgen-typescript-loader to react-docgen-typescript-plugin #11106

Merged
merged 3 commits into from
Jun 11, 2020
Merged

React: Switch react-docgen-typescript-loader to react-docgen-typescript-plugin #11106

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4286a93
switch from react-docgen-typescript-loader to react-docgen-typescript…
hipstersmoothie Jun 9, 2020
09cbb9a
dont use babel plugin if using typescript docgen
hipstersmoothie Jun 9, 2020
47f0f0d
update perf stats
hipstersmoothie Jun 9, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion addons/docs/react/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ Neither option is perfect, so here's everything you should know if you're thinki

| Docgen | Build time |
| ----------------------- | ---------- |
| react-docgen-typescript | 59s |
| react-docgen-typescript | 33s |
| react-docgen | 29s |
| none | 28s |

Expand Down
3 changes: 1 addition & 2 deletions addons/docs/src/lib/sbtypes/proptypes/convert.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,7 @@ export const convert = (type: PTType): SBType | any => {
case 'elementType':
default: {
if (name?.indexOf('|') > 0) {
// react-docgen-typescript-loader doesn't always produce proper
// enum types, possibly due to https://github.com/strothj/react-docgen-typescript-loader/issues/81
// react-docgen-typescript-plugin doesn't always produce proper
Copy link
Member

Choose a reason for hiding this comment

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

This comment seems to have only been partially removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This comment relates to the issue @shilman commented on. Will try to resolve this in the underlying library and remove this comment entirely

Copy link
Member

Choose a reason for hiding this comment

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

I understand, but one line was removed and now it doesn't make sense, right?

// react-docgen-typescript-plugin doesn't always produce proper
// this hack tries to parse out values from the string and should be
// removed when RDTL gets a little smarter about this

// this hack tries to parse out values from the string and should be
// removed when RDTL gets a little smarter about this
try {
Expand Down
2 changes: 1 addition & 1 deletion app/react/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
"lodash": "^4.17.15",
"prop-types": "^15.7.2",
"react-dev-utils": "^10.0.0",
"react-docgen-typescript-loader": "^3.7.2",
"react-docgen-typescript-plugin": "^0.0.2",
"regenerator-runtime": "^0.13.3",
"ts-dedent": "^1.1.1",
"webpack": "^4.43.0"
Expand Down
28 changes: 10 additions & 18 deletions app/react/src/server/framework-preset-react-docgen.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
import type { TransformOptions } from '@babel/core';
import type { Configuration } from 'webpack';
import type { StorybookOptions } from '@storybook/core/types';
import ReactDocgenTypescriptPlugin from 'react-docgen-typescript-plugin';

export function babel(config: TransformOptions, { typescriptOptions }: StorybookOptions) {
const { reactDocgen } = typescriptOptions;
if (!reactDocgen) {

if (!reactDocgen || reactDocgen === 'react-docgen-typescript') {
return config;
}

return {
...config,
overrides: [
Expand All @@ -27,24 +30,13 @@ export function babel(config: TransformOptions, { typescriptOptions }: Storybook

export function webpackFinal(config: Configuration, { typescriptOptions }: StorybookOptions) {
const { reactDocgen, reactDocgenTypescriptOptions } = typescriptOptions;
if (reactDocgen !== 'react-docgen-typescript') return config;

if (reactDocgen !== 'react-docgen-typescript') {
Copy link
Member

Choose a reason for hiding this comment

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

Why was this changed?

Copy link
Member

Choose a reason for hiding this comment

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

I personally prefer the new code

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 can change it back but one line if statement throw me for a loop. the function above this one also had this style of if statement. Can change back if wanted.

Copy link
Member

Choose a reason for hiding this comment

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

I was more pointing out that this change isn't related to any of the work. I think it's good practice to try to only change things related to the task at hand, unless something is being improved. But if you think the change is meaningful, then I'm OK with that.

return config;
}

return {
...config,
module: {
...config.module,
rules: [
...config.module.rules,
{
test: /\.tsx?$/,
// include: path.resolve(__dirname, "../src"),
use: [
{
loader: require.resolve('react-docgen-typescript-loader'),
options: reactDocgenTypescriptOptions,
},
],
},
],
},
plugins: [...config.plugins, new ReactDocgenTypescriptPlugin(reactDocgenTypescriptOptions)],
};
}
14 changes: 7 additions & 7 deletions docs/src/pages/configurations/typescript-config/index.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ Each framework uses the base configuration unless otherwise specified:

- **Angular** ignores the base and uses `ts-loader` and `ngx-template-loader`.
- **Vue** ignores the uses `ts-loader` and applies it to both `.tsx?` and `.vue` files.
- **React** adds `react-docgen-typescript-loader` the base.
- **React** adds `react-docgen-typescript-plugin` the base.

## Main.js configuration

Expand All @@ -42,12 +42,12 @@ module.exports = {

And here are the meaning of each field:

| Field | Framework | Description | Type |
| -------------------------------- | --------- | -------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------ |
| **check** | All | optionally run `fork-ts-checker-webpack-plugin` | `boolean` |
| **checkOptions** | All | Options to pass to `fork-ts-checker-webpack-plugin` if it's enabled | [See docs](https://github.com/TypeStrong/fork-ts-checker-webpack-plugin) |
| **reactDocgen** | React | which variant docgen processor to run | `'react-docgen-typescript' | 'react-docgen' | false` |
| **reactDocgenTypescriptOptions** | React | Options to pass to `react-docgen-typescript-loader` if `react-docgen-typescript` is enabled. | [See docs](https://github.com/strothj/react-docgen-typescript-loader) |
| Field | Framework | Description | Type |
| -------------------------------- | --------- | -------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------- |
| **check** | All | optionally run `fork-ts-checker-webpack-plugin` | `boolean` |
| **checkOptions** | All | Options to pass to `fork-ts-checker-webpack-plugin` if it's enabled | [See docs](https://github.com/TypeStrong/fork-ts-checker-webpack-plugin) |
| **reactDocgen** | React | which variant docgen processor to run | `'react-docgen-typescript' | 'react-docgen' | false` |
| **reactDocgenTypescriptOptions** | React | Options to pass to `react-docgen-typescript-plugin` if `react-docgen-typescript` is enabled. | [See docs](https://github.com/hipstersmoothie/react-docgen-typescript-plugin) |

## Manual configuration

Expand Down
2 changes: 1 addition & 1 deletion examples/cra-ts-kitchen-sink/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
"enzyme-adapter-react-16": "^1.9.1",
"enzyme-to-json": "^3.4.1",
"fork-ts-checker-webpack-plugin": "^4.0.3",
"react-docgen-typescript-loader": "^3.7.2",
"react-docgen-typescript-plugin": "^0.0.2",
"react-moment-proptypes": "^1.7.0",
"ts-node": "~8.9.1"
},
Expand Down
6 changes: 3 additions & 3 deletions lib/core/types/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type ForkTsCheckerWebpackPlugin from 'fork-ts-checker-webpack-plugin';
import type LoaderOptions from 'react-docgen-typescript-loader/dist/LoaderOptions';
import type { PluginOptions } from 'react-docgen-typescript-plugin';
import { Configuration } from 'webpack';

/**
Expand Down Expand Up @@ -60,10 +60,10 @@ export interface TypescriptOptions {
*/
reactDocgen: 'react-docgen-typescript' | 'react-docgen' | false;
/**
* Configures `react-docgen-typescript-loader`
* Configures `react-docgen-typescript-plugin`
*
* @default
* @see https://github.com/storybookjs/storybook/blob/next/lib/core/src/server/config/defaults.js#L4-L6
*/
reactDocgenTypescriptOptions: LoaderOptions;
reactDocgenTypescriptOptions: PluginOptions;
}
39 changes: 39 additions & 0 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13398,6 +13398,15 @@ end-of-stream@^1.0.0, end-of-stream@^1.1.0:
dependencies:
once "^1.4.0"

endent@^2.0.1:
version "2.0.1"
resolved "https://registry.yarnpkg.com/endent/-/endent-2.0.1.tgz#fb18383a3f37ae3213a5d9f6c4a880d1061eb4c5"
integrity sha512-mADztvcC+vCk4XEZaCz6xIPO2NHQuprv5CAEjuVAu6aZwqAj7nVNlMyl1goPFYqCCpS2OJV9jwpumJLkotZrNw==
dependencies:
dedent "^0.7.0"
fast-json-parse "^1.0.3"
objectorarray "^1.0.4"

engine.io-client@~3.4.0:
version "3.4.2"
resolved "https://registry.yarnpkg.com/engine.io-client/-/engine.io-client-3.4.2.tgz#4fb2ef2b1fe1d3aa1c621c6a8d87f1fc55426b50"
Expand Down Expand Up @@ -14769,6 +14778,11 @@ fast-glob@^3.0.3, fast-glob@^3.1.0, fast-glob@^3.1.1:
micromatch "^4.0.2"
picomatch "^2.2.1"

fast-json-parse@^1.0.3:
version "1.0.3"
resolved "https://registry.yarnpkg.com/fast-json-parse/-/fast-json-parse-1.0.3.tgz#43e5c61ee4efa9265633046b770fb682a7577c4d"
integrity sha512-FRWsaZRWEJ1ESVNbDWmsAlqDk96gPQezzLghafp5J4GUKjbCz3OkAHuZs5TuPEtkbVQERysLp9xv6c24fBm8Aw==

fast-json-patch@^3.0.0-1:
version "3.0.0-1"
resolved "https://registry.yarnpkg.com/fast-json-patch/-/fast-json-patch-3.0.0-1.tgz#4c68f2e7acfbab6d29d1719c44be51899c93dabb"
Expand Down Expand Up @@ -23821,6 +23835,11 @@ object.values@^1.1.0, object.values@^1.1.1:
function-bind "^1.1.1"
has "^1.0.3"

objectorarray@^1.0.4:
version "1.0.4"
resolved "https://registry.yarnpkg.com/objectorarray/-/objectorarray-1.0.4.tgz#d69b2f0ff7dc2701903d308bb85882f4ddb49483"
integrity sha512-91k8bjcldstRz1bG6zJo8lWD7c6QXcB4nTDUqiEvIL1xAsLoZlOOZZG+nd6YPz+V7zY1580J4Xxh1vZtyv4i/w==

obuf@^1.0.0, obuf@^1.1.2:
version "1.1.2"
resolved "https://registry.yarnpkg.com/obuf/-/obuf-1.1.2.tgz#09bea3343d41859ebd446292d11c9d4db619084e"
Expand Down Expand Up @@ -27098,11 +27117,26 @@ react-docgen-typescript-loader@^3.7.2:
loader-utils "^1.2.3"
react-docgen-typescript "^1.15.0"

react-docgen-typescript-plugin@^0.0.2:
version "0.0.2"
resolved "https://registry.yarnpkg.com/react-docgen-typescript-plugin/-/react-docgen-typescript-plugin-0.0.2.tgz#80aa25b5195cc2be1b21b734becaf71cccfb5208"
integrity sha512-8M6bECmvc9T5uW8lBt79+EZC9IYx7fSApWxtWPq7awqPwCecxd2xIay1m/9kBk+DfpShgxU+5fpdviPVBWxtdQ==
dependencies:
endent "^2.0.1"
react-docgen-typescript "^1.16.6"
react-docgen-typescript-loader "^3.7.2"
tslib "^2.0.0"

react-docgen-typescript@^1.15.0:
version "1.16.5"
resolved "https://registry.yarnpkg.com/react-docgen-typescript/-/react-docgen-typescript-1.16.5.tgz#b305563425ab370f5a3c82b42579eb5069449b87"
integrity sha512-guXnx6a554IDVUoVIkX/BGRTrwc2n2w/kMxo7TKLNLJW1qszhT6BRHX4qV8eWq5eaJxRxuesOW5AOLiOI9WQOA==

react-docgen-typescript@^1.16.6:
version "1.16.6"
resolved "https://registry.yarnpkg.com/react-docgen-typescript/-/react-docgen-typescript-1.16.6.tgz#334c1b346cdaba9a347c7395fa4fc37a3a3292b5"
integrity sha512-Nt+Oireji1q/6ZV7LAZHLJfd86gpYSiuX519MTgnPbyXsTTK5iCYm5SZz1bkD42xTK7nXZen2UicY2xER3J3Nw==

react-docgen@^5.0.0:
version "5.3.0"
resolved "https://registry.yarnpkg.com/react-docgen/-/react-docgen-5.3.0.tgz#9aabde5e69f1993c8ba839fd9a86696504654589"
Expand Down Expand Up @@ -31774,6 +31808,11 @@ tslib@^1.10.0, tslib@^1.11.1, tslib@^1.8.0, tslib@^1.8.1, tslib@^1.9.0:
resolved "https://registry.yarnpkg.com/tslib/-/tslib-1.13.0.tgz#c881e13cc7015894ed914862d276436fa9a47043"
integrity sha512-i/6DQjL8Xf3be4K/E6Wgpekn5Qasl1usyw++dAA35Ue5orEn65VIxOA+YvNNl9HV3qv70T7CNwjODHZrLwvd1Q==

tslib@^2.0.0:
version "2.0.0"
resolved "https://registry.yarnpkg.com/tslib/-/tslib-2.0.0.tgz#18d13fc2dce04051e20f074cc8387fd8089ce4f3"
integrity sha512-lTqkx847PI7xEDYJntxZH89L2/aXInsyF2luSafe/+0fHOMjlBNXdH6th7f70qxLDhul7KZK0zC8V5ZIyHl0/g==

tslint@^5.20.1:
version "5.20.1"
resolved "https://registry.yarnpkg.com/tslint/-/tslint-5.20.1.tgz#e401e8aeda0152bc44dd07e614034f3f80c67b7d"
Expand Down