-
-
Notifications
You must be signed in to change notification settings - Fork 9.8k
React: Extract jsdoc tags in component manifest #32748
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
Changes from all commits
67d5780
5efe183
446abe5
ef96eff
2d86537
3738a60
2442574
18aaaf4
50ea146
addd229
4ed3856
a449847
4ce972a
1599fac
1bcd00b
bf9a49a
f2e3ddb
c0df22c
25b197e
18c671e
11ac914
98f121b
bd91ab6
d3f1940
b865df9
51cdfc7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -8,6 +8,7 @@ import polka from 'polka'; | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import invariant from 'tiny-invariant'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { telemetry } from '../telemetry'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { type ComponentManifestGenerator } from '../types'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import type { StoryIndexGenerator } from './utils/StoryIndexGenerator'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { doTelemetry } from './utils/doTelemetry'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { getManagerBuilder, getPreviewBuilder } from './utils/get-builders'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -135,6 +136,32 @@ export async function storybookDevServer(options: Options) { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw indexError; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| app.use('/manifests/components.json', async (req, res) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const features = await options.presets.apply('features'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!features?.experimental_componentsManifest) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const componentManifestGenerator: ComponentManifestGenerator = await options.presets.apply( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 'componentManifestGenerator' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const indexGenerator = await initializedStoryIndexGenerator; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (componentManifestGenerator && indexGenerator) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const manifest = await componentManifestGenerator(indexGenerator); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| res.setHeader('Content-Type', 'application/json'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| res.end(JSON.stringify(manifest)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| res.statusCode = 400; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| res.end('No component manifest generator configured.'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (e) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger.error(e instanceof Error ? e : String(e)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| res.statusCode = 500; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| res.end(e instanceof Error ? e.toString() : String(e)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+139
to
+163
Contributor
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. Fix inverted feature flag logic. Line 142 checks Apply this diff to fix the logic: const features = await options.presets.apply('features');
- if (!features?.experimental_componentsManifest) {
+ if (features?.experimental_componentsManifest) {
const componentManifestGenerator: ComponentManifestGenerator = await options.presets.apply(
'componentManifestGenerator'
);📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Now the preview has successfully started, we can count this as a 'dev' event. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| doTelemetry(app, core, initializedStoryIndexGenerator, options); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| const { fs } = require('memfs'); | ||
|
|
||
| module.exports = fs.promises; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| /** | ||
| * This is heavily based on the react-docgen `displayNameHandler` | ||
| * (https://github.com/reactjs/react-docgen/blob/26c90c0dd105bf83499a83826f2a6ff7a724620d/src/handlers/displayNameHandler.ts) | ||
| * but instead defines an `actualName` property on the generated docs that is taken first from the | ||
| * component's actual name. This addresses an issue where the name that the generated docs are | ||
| * stored under is incorrectly named with the `displayName` and not the component's actual name. | ||
| * | ||
| * This is inspired by `actualNameHandler` from | ||
| * https://github.com/storybookjs/babel-plugin-react-docgen, but is modified directly from | ||
| * displayNameHandler, using the same approach as babel-plugin-react-docgen. | ||
| */ | ||
| import type { Handler, NodePath, babelTypes as t } from 'react-docgen'; | ||
| import { utils } from 'react-docgen'; | ||
|
|
||
| const { getNameOrValue, isReactForwardRefCall } = utils; | ||
|
|
||
| const actualNameHandler: Handler = function actualNameHandler(documentation, componentDefinition) { | ||
| documentation.set('definedInFile', componentDefinition.hub.file.opts.filename); | ||
|
|
||
| if ( | ||
| (componentDefinition.isClassDeclaration() || componentDefinition.isFunctionDeclaration()) && | ||
| componentDefinition.has('id') | ||
| ) { | ||
| documentation.set( | ||
| 'actualName', | ||
| getNameOrValue(componentDefinition.get('id') as NodePath<t.Identifier>) | ||
| ); | ||
| } else if ( | ||
| componentDefinition.isArrowFunctionExpression() || | ||
| componentDefinition.isFunctionExpression() || | ||
| isReactForwardRefCall(componentDefinition) | ||
| ) { | ||
| let currentPath: NodePath = componentDefinition; | ||
|
|
||
| while (currentPath.parentPath) { | ||
| if (currentPath.parentPath.isVariableDeclarator()) { | ||
| documentation.set('actualName', getNameOrValue(currentPath.parentPath.get('id'))); | ||
| return; | ||
| } | ||
| if (currentPath.parentPath.isAssignmentExpression()) { | ||
| const leftPath = currentPath.parentPath.get('left'); | ||
|
|
||
| if (leftPath.isIdentifier() || leftPath.isLiteral()) { | ||
| documentation.set('actualName', getNameOrValue(leftPath)); | ||
| return; | ||
| } | ||
| } | ||
|
|
||
| currentPath = currentPath.parentPath; | ||
| } | ||
| // Could not find an actual name | ||
| documentation.set('actualName', ''); | ||
| } | ||
| }; | ||
|
|
||
| export default actualNameHandler; |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,75 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { extname } from 'node:path'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import resolve from 'resolve'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export class ReactDocgenResolveError extends Error { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // the magic string that react-docgen uses to check if a module is ignored | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| readonly code = 'MODULE_NOT_FOUND'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| constructor(filename: string) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| super(`'${filename}' was ignored by react-docgen.`); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /* The below code was copied from: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * https://github.com/reactjs/react-docgen/blob/df2daa8b6f0af693ecc3c4dc49f2246f60552bcb/packages/react-docgen/src/importer/makeFsImporter.ts#L14-L63 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * because it wasn't exported from the react-docgen package. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * watch out: when updating this code, also update the code in code/presets/react-webpack/src/loaders/docgen-resolver.ts | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // These extensions are sorted by priority | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // resolve() will check for files in the order these extensions are sorted | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export const RESOLVE_EXTENSIONS = [ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| '.js', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| '.cts', // These were originally not in the code, I added them | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| '.mts', // These were originally not in the code, I added them | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| '.ctsx', // These were originally not in the code, I added them | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| '.mtsx', // These were originally not in the code, I added them | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| '.ts', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| '.tsx', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| '.mjs', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| '.cjs', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| '.mts', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| '.cts', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| '.jsx', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+22
to
+35
Contributor
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. Duplicate extension entries detected. Lines 25-27 and 32-33 both include Apply this diff to remove the duplicates: export const RESOLVE_EXTENSIONS = [
'.js',
'.cts', // These were originally not in the code, I added them
'.mts', // These were originally not in the code, I added them
'.ctsx', // These were originally not in the code, I added them
'.mtsx', // These were originally not in the code, I added them
'.ts',
'.tsx',
'.mjs',
'.cjs',
- '.mts',
- '.cts',
'.jsx',
];📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export function defaultLookupModule(filename: string, basedir: string): string { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const resolveOptions = { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| basedir, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| extensions: RESOLVE_EXTENSIONS, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // we do not need to check core modules as we cannot import them anyway | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| includeCoreModules: false, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return resolve.sync(filename, resolveOptions); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const ext = extname(filename); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let newFilename: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // if we try to import a JavaScript file it might be that we are actually pointing to | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // a TypeScript file. This can happen in ES modules as TypeScript requires to import other | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // TypeScript files with .js extensions | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // https://www.typescriptlang.org/docs/handbook/esm-node.html#type-in-packagejson-and-new-extensions | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| switch (ext) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| case '.js': | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| case '.mjs': | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| case '.cjs': | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| newFilename = `${filename.slice(0, -2)}ts`; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| break; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| case '.jsx': | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| newFilename = `${filename.slice(0, -3)}tsx`; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| break; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| default: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw error; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return resolve.sync(newFilename, { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ...resolveOptions, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // we already know that there is an extension at this point, so no need to check other extensions | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| extensions: [], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
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.
Remove shadowing variable and use existing
features.Line 168 creates a shadow variable
featuresthat was already defined and fetched on line 101. This is redundant and can lead to confusion.Apply this diff to use the existing
featuresvariable:📝 Committable suggestion
🤖 Prompt for AI Agents