-
Notifications
You must be signed in to change notification settings - Fork 28
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 test case for sub components (demonstrates invalid syntax in output) #60
Closed
stevenpetryk
wants to merge
10
commits into
hipstersmoothie:master
from
stevenpetryk:sub-component-error-repro
Closed
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
b743c7d
fix: Restore old webpack 4 code path to fix hmr after refresh
bebraw c2a43fb
chore: Drop a redundant dependency
bebraw 09fe9dc
fix: Fix a lint error
bebraw bb8fd0f
fix: Add missing getModuleEvaluationSideEffectsState
bebraw f24f46c
fix: Fix lint error
bebraw 98f9b13
Merge branch 'pr/46' into next
shilman 3e13e62
fix: Fix rebuild performance for webpack 5
bebraw 4cdd013
Merge pull request #1 from storybookjs/fix/rebuild-in-wp5
shilman 74bb959
Fix CI branches
shilman 1d84508
Create test case showing invalid syntax issue
stevenpetryk File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
import * as React from "react"; | ||
|
||
export default function Root(props: { name: string }) { | ||
return <span>root {props.name}</span>; | ||
} | ||
|
||
function Sub(props: { name: string }) { | ||
return <span>sub {props.name}</span>; | ||
} | ||
|
||
Root.Sub = Sub; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -209,6 +209,35 @@ try { | |
catch (__react_docgen_typescript_loader_error) { }" | ||
`; | ||
|
||
exports[`component fixture SubComponent.tsx has code block generated 1`] = ` | ||
"import * as React from \\"react\\"; | ||
|
||
export default function Root(props: { name: string }) { | ||
return <span>root {props.name}</span>; | ||
} | ||
|
||
function Sub(props: { name: string }) { | ||
return <span>sub {props.name}</span>; | ||
} | ||
|
||
Root.Sub = Sub; | ||
|
||
try { | ||
// @ts-ignore | ||
SubComponent.displayName = \\"SubComponent\\"; | ||
// @ts-ignore | ||
SubComponent.__docgenInfo = { \\"description\\": \\"\\", \\"displayName\\": \\"SubComponent\\", \\"props\\": { \\"name\\": { \\"defaultValue\\": null, \\"description\\": \\"\\", \\"name\\": \\"name\\", \\"required\\": true, \\"type\\": { \\"name\\": \\"string\\" } } } }; | ||
} | ||
catch (__react_docgen_typescript_loader_error) { } | ||
try { | ||
// @ts-ignore | ||
default.Sub.displayName = \\"default.Sub\\"; | ||
// @ts-ignore | ||
default.Sub.__docgenInfo = { \\"description\\": \\"\\", \\"displayName\\": \\"default.Sub\\", \\"props\\": { \\"name\\": { \\"defaultValue\\": null, \\"description\\": \\"\\", \\"name\\": \\"name\\", \\"required\\": true, \\"type\\": { \\"name\\": \\"string\\" } } } }; | ||
Comment on lines
+234
to
+236
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Syntax errors here (so even the defensive |
||
} | ||
catch (__react_docgen_typescript_loader_error) { }" | ||
`; | ||
|
||
exports[`component fixture TextOnlyComponent.tsx has code block generated 1`] = ` | ||
"import * as React from \\"react\\"; | ||
|
||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -143,6 +143,18 @@ export default class DocgenPlugin implements webpack.WebpackPluginInstance { | |
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Irrelevant file, sorry again! |
||
|
||
apply(compiler: webpack.Compiler): void { | ||
// Property compiler.version is set only starting from webpack 5 | ||
const webpackVersion = compiler.webpack?.version || ""; | ||
const isWebpack5 = parseInt(webpackVersion.split(".")[0], 10) >= 5; | ||
|
||
if (isWebpack5) { | ||
this.applyWebpack5(compiler); | ||
} else { | ||
this.applyWebpack4(compiler); | ||
} | ||
} | ||
|
||
applyWebpack5(compiler: webpack.Compiler): void { | ||
const pluginName = "DocGenPlugin"; | ||
const { | ||
docgenOptions, | ||
|
@@ -156,29 +168,24 @@ export default class DocgenPlugin implements webpack.WebpackPluginInstance { | |
const { exclude = [], include = ["**/**.tsx"] } = this.options; | ||
const isExcluded = matchGlob(exclude); | ||
const isIncluded = matchGlob(include); | ||
// Property compiler.version is set only starting from webpack 5 | ||
const webpackVersion = compiler.webpack?.version || ""; | ||
const isWebpack5 = parseInt(webpackVersion.split(".")[0], 10) >= 5; | ||
|
||
compiler.hooks.compilation.tap( | ||
pluginName, | ||
(compilation: webpack.Compilation) => { | ||
if (isWebpack5) { | ||
// Since this file is needed only for webpack 5, load it only then | ||
// to simplify the implementation of the file. | ||
// | ||
// eslint-disable-next-line | ||
const { DocGenDependency } = require("./dependency"); | ||
// Since this file is needed only for webpack 5, load it only then | ||
// to simplify the implementation of the file. | ||
// | ||
// eslint-disable-next-line | ||
const { DocGenDependency } = require("./dependency"); | ||
|
||
compilation.dependencyTemplates.set( | ||
// eslint-disable-next-line | ||
// @ts-ignore: Webpack 4 type | ||
DocGenDependency, | ||
// eslint-disable-next-line | ||
// @ts-ignore: Webpack 4 type | ||
new DocGenDependency.Template() | ||
); | ||
} | ||
compilation.dependencyTemplates.set( | ||
// eslint-disable-next-line | ||
// @ts-ignore: Webpack 4 type | ||
DocGenDependency, | ||
// eslint-disable-next-line | ||
// @ts-ignore: Webpack 4 type | ||
new DocGenDependency.Template() | ||
); | ||
|
||
compilation.hooks.seal.tap(pluginName, () => { | ||
const modulesToProcess: [string, webpack.Module][] = []; | ||
|
@@ -191,6 +198,30 @@ export default class DocgenPlugin implements webpack.WebpackPluginInstance { | |
|
||
const nameForCondition = module.nameForCondition() || ""; | ||
|
||
// Ignore already built modules for webpack 5 | ||
if (!compilation.builtModules.has(module)) { | ||
debugExclude(`Ignoring un-built module: ${nameForCondition}`); | ||
return; | ||
} | ||
|
||
// Ignore external modules | ||
// eslint-disable-next-line | ||
// @ts-ignore: Webpack 4 type | ||
if (module.external) { | ||
debugExclude(`Ignoring external module: ${nameForCondition}`); | ||
return; | ||
} | ||
|
||
// Ignore raw requests | ||
// eslint-disable-next-line | ||
// @ts-ignore: Webpack 4 type | ||
if (!module.rawRequest) { | ||
debugExclude( | ||
`Ignoring module without "rawRequest": ${nameForCondition}` | ||
); | ||
return; | ||
} | ||
|
||
if (isExcluded(nameForCondition)) { | ||
debugExclude( | ||
`Module not matched in "exclude": ${nameForCondition}` | ||
|
@@ -217,38 +248,122 @@ export default class DocgenPlugin implements webpack.WebpackPluginInstance { | |
// 3. Process and parse each module and add the type information | ||
// as a dependency | ||
modulesToProcess.forEach(([name, module]) => { | ||
if (isWebpack5) { | ||
// Since this file is needed only for webpack 5, load it only then | ||
// to simplify the implementation of the file. | ||
// | ||
// Since this file is needed only for webpack 5, load it only then | ||
// to simplify the implementation of the file. | ||
// | ||
// eslint-disable-next-line | ||
const { DocGenDependency } = require("./dependency"); | ||
|
||
module.addDependency( | ||
// eslint-disable-next-line | ||
const { DocGenDependency } = require("./dependency"); | ||
|
||
module.addDependency( | ||
// eslint-disable-next-line | ||
// @ts-ignore: Webpack 4 type | ||
new DocGenDependency( | ||
generateDocgenCodeBlock({ | ||
filename: name, | ||
source: name, | ||
componentDocs: docGenParser.parseWithProgramProvider( | ||
name, | ||
() => tsProgram | ||
), | ||
...generateOptions, | ||
}).substring(name.length) | ||
) | ||
); | ||
} else { | ||
// Assume webpack 4 or earlier | ||
processModule(docGenParser, module, tsProgram, generateOptions); | ||
} | ||
// @ts-ignore: Webpack 4 type | ||
new DocGenDependency( | ||
generateDocgenCodeBlock({ | ||
filename: name, | ||
source: name, | ||
componentDocs: docGenParser.parseWithProgramProvider( | ||
name, | ||
() => tsProgram | ||
), | ||
...generateOptions, | ||
}).substring(name.length) | ||
) | ||
); | ||
}); | ||
}); | ||
} | ||
); | ||
} | ||
|
||
applyWebpack4(compiler: webpack.Compiler): void { | ||
const { docgenOptions, compilerOptions } = this.getOptions(); | ||
const parser = docGen.withCompilerOptions(compilerOptions, docgenOptions); | ||
const { exclude = [], include = ["**/**.tsx"] } = this.options; | ||
const isExcluded = matchGlob(exclude); | ||
const isIncluded = matchGlob(include); | ||
|
||
compiler.hooks.make.tap(this.name, (compilation) => { | ||
compilation.hooks.seal.tap(this.name, () => { | ||
const modulesToProcess: webpack.Module[] = []; | ||
|
||
compilation.modules.forEach((module: webpack.Module) => { | ||
// eslint-disable-next-line | ||
// @ts-ignore: Webpack 4 type | ||
if (!module.built) { | ||
// eslint-disable-next-line | ||
// @ts-ignore: Webpack 4 type | ||
debugExclude(`Ignoring un-built module: ${module.userRequest}`); | ||
return; | ||
} | ||
|
||
// eslint-disable-next-line | ||
// @ts-ignore: Webpack 4 type | ||
if (module.external) { | ||
// eslint-disable-next-line | ||
// @ts-ignore: Webpack 4 type | ||
debugExclude(`Ignoring external module: ${module.userRequest}`); | ||
return; | ||
} | ||
|
||
// eslint-disable-next-line | ||
// @ts-ignore: Webpack 4 type | ||
if (!module.rawRequest) { | ||
debugExclude( | ||
// eslint-disable-next-line | ||
// @ts-ignore: Webpack 4 type | ||
`Ignoring module without "rawRequest": ${module.userRequest}` | ||
); | ||
return; | ||
} | ||
|
||
// eslint-disable-next-line | ||
// @ts-ignore: Webpack 4 type | ||
if (isExcluded(module.userRequest)) { | ||
debugExclude( | ||
// eslint-disable-next-line | ||
// @ts-ignore: Webpack 4 type | ||
`Module not matched in "exclude": ${module.userRequest}` | ||
); | ||
return; | ||
} | ||
|
||
// eslint-disable-next-line | ||
// @ts-ignore: Webpack 4 type | ||
if (!isIncluded(module.userRequest)) { | ||
debugExclude( | ||
// eslint-disable-next-line | ||
// @ts-ignore: Webpack 4 type | ||
`Module not matched in "include": ${module.userRequest}` | ||
); | ||
return; | ||
} | ||
|
||
// eslint-disable-next-line | ||
// @ts-ignore: Webpack 4 type | ||
debugInclude(module.userRequest); | ||
modulesToProcess.push(module); | ||
}); | ||
|
||
const tsProgram = ts.createProgram( | ||
// eslint-disable-next-line | ||
// @ts-ignore: Webpack 4 type | ||
modulesToProcess.map((v) => v.userRequest), | ||
compilerOptions | ||
); | ||
|
||
modulesToProcess.forEach((m) => | ||
processModule(parser, m, tsProgram, { | ||
docgenCollectionName: "STORYBOOK_REACT_CLASSES", | ||
setDisplayName: true, | ||
typePropName: "type", | ||
}) | ||
); | ||
|
||
cache.save(); | ||
}); | ||
}); | ||
} | ||
|
||
getOptions(): { | ||
docgenOptions: docGen.ParserOptions; | ||
generateOptions: { | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Sorry, I think I cloned Storybook's fork of this project. Some of these changes are unrelated. But check out the snapshot below :)