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

Use a central script instead of one per package to generate docs #14216

Merged
merged 7 commits into from
Mar 5, 2019
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
29 changes: 29 additions & 0 deletions bin/update-readmes.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
#!/usr/bin/env node

const path = require( 'path' );
const childProcess = require( 'child_process' );

const packages = [
'e2e-test-utils',
];

let aggregatedExitCode = 0;
packages.forEach( ( packageName ) => {
const args = [
`packages/${ packageName }/src/index.js`,
`--output packages/${ packageName }/README.md`,
'--to-token',
];
const pathToDocGen = path.join( __dirname, '..', 'node_modules', '.bin', 'docgen' );
const { status, stderr } = childProcess.spawnSync(
Copy link
Member

Choose a reason for hiding this comment

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

We've a bad habit in our scripts of making them blocking/synchronous when they don't need to be.

See #14295 for a proposed revision.

pathToDocGen,
args,
{ shell: true },
);
if ( status !== 0 ) {
aggregatedExitCode = status;
process.stderr.write( `${ stderr }\n` );
}
} );

process.exit( aggregatedExitCode );
8 changes: 5 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -162,8 +162,7 @@
"predev": "npm run check-engines",
"dev": "npm run build:packages && concurrently \"wp-scripts start\" \"npm run dev:packages\"",
"dev:packages": "node ./bin/packages/watch.js",
"docs:build": "node docs/tool",
"docs:generate": "lerna run docs:generate",
"docs:build": "node ./docs/tool && node ./bin/update-readmes",
"fixtures:clean": "rimraf \"packages/e2e-tests/fixtures/blocks/*.+(json|serialized.html)\"",
"fixtures:server-registered": "docker-compose run -w /var/www/html/wp-content/plugins/gutenberg --rm wordpress ./bin/get-server-blocks.php > test/integration/full-content/server-registered.json",
"fixtures:generate": "npm run fixtures:server-registered && cross-env GENERATE_MISSING_FIXTURES=y npm run test-unit",
Expand Down Expand Up @@ -204,7 +203,10 @@
"wp-scripts lint-js"
],
"{docs/{toc.json,tool/*.js},packages/{*/README.md,*/src/{actions,selectors}.js,components/src/*/**/README.md}}": [
"npm run docs:build"
"node ./docs/tool"
],
"packages/**/*.js": [
Copy link
Member Author

Choose a reason for hiding this comment

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

I wanted to ignore files related to tests, but it looks like I can't do that (ignore option affects all tasks).

Copy link
Member Author

Choose a reason for hiding this comment

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

Also note that although the script updates some README file, the exit code will be 0 so it won't prevent the commit from happening. The docgen package doesn't have any knowledge of whether the output file has been modified with regards to a previous version, so if we strongly wanted to have that it should be done in a further PR implementing some kind of git command.

"node ./bin/update-readmes"
]
}
}
2 changes: 1 addition & 1 deletion packages/docgen/bin/cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,4 +41,4 @@ const optionator = require( 'optionator' )( {
} );

const options = optionator.parseArgv( process.argv );
docgen( options._[ 0 ], options );
process.exit( docgen( options._[ 0 ], options ) );
26 changes: 14 additions & 12 deletions packages/docgen/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,11 @@ const relativeToAbsolute = ( basePath, relativePath ) => {
if ( fs.existsSync( targetFile ) ) {
return targetFile;
}
process.stdout.write( '\nRelative path does not exists.' );
process.stdout.write( '\n' );
process.stdout.write( `\nBase: ${ basePath }` );
process.stdout.write( `\nRelative: ${ relativePath }` );
process.stdout.write( '\n\n' );
process.stderr.write( '\nRelative path does not exists.' );
process.stderr.write( '\n' );
process.stderr.write( `\nBase: ${ basePath }` );
process.stderr.write( `\nRelative: ${ relativePath }` );
process.stderr.write( '\n\n' );
process.exit( 1 );
};

Expand All @@ -54,8 +54,8 @@ const processFile = ( rootDir, inputFile ) => {
currentFileStack.pop( inputFile );
return result;
} catch ( e ) {
process.stdout.write( `\n${ e }` );
process.stdout.write( '\n\n' );
process.stderr.write( `\n${ e }` );
process.stderr.write( '\n\n' );
process.exit( 1 );
}
};
Expand All @@ -66,8 +66,8 @@ const runCustomFormatter = ( customFormatterFile, rootDir, doc, symbols, heading
const output = customFormatter( rootDir, doc, symbols, headingTitle );
fs.writeFileSync( doc, output );
} catch ( e ) {
process.stdout.write( `\n${ e }` );
process.stdout.write( '\n\n' );
process.stderr.write( `\n${ e }` );
process.stderr.write( '\n\n' );
process.exit( 1 );
}
return 'custom formatter';
Expand All @@ -80,9 +80,9 @@ module.exports = function( sourceFile, options ) {
// Input: process CLI args, prepare files, etc
const processDir = process.cwd();
if ( sourceFile === undefined ) {
process.stdout.write( '\n' );
process.stdout.write( 'No source file provided' );
process.stdout.write( '\n\n' );
process.stderr.write( '\n' );
process.stderr.write( 'No source file provided' );
process.stderr.write( '\n\n' );
process.exit( 1 );
}
sourceFile = path.join( processDir, sourceFile );
Expand Down Expand Up @@ -123,4 +123,6 @@ module.exports = function( sourceFile, options ) {
fs.writeFileSync( tokens, JSON.stringify( result.tokens ) );
fs.writeFileSync( ast, JSON.stringify( result.ast ) );
}

process.exit( 0 );
};
6 changes: 0 additions & 6 deletions packages/e2e-test-utils/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,17 +33,11 @@
"lodash": "^4.17.11",
"node-fetch": "^1.7.3"
},
"devDependencies": {
"@wordpress/docgen": "file:../docgen"
},
"peerDependencies": {
"jest": ">=24",
"puppeteer": ">=1.6"
},
"publishConfig": {
"access": "public"
},
"scripts": {
"docs:generate": "docgen ./src/index.js --output ./README.md --to-token"
}
}