Skip to content

Commit

Permalink
Add script which checks published types for non-checked JS packages (#…
Browse files Browse the repository at this point in the history
…49736)

* Adds script which verifies that published build-types are actually correct.
  Necessary for packages which disabled JS type-checking in tsc, which can
  result in errors from JSDoc making their way into published types. This runs
  locally and in CI when building package types.
* Update rememo dependency to resolve an existing error in build types.
* Allow react-native metro config to resolve .cjs files. (Necessary for rememo update.)
  • Loading branch information
noahtallen authored Apr 19, 2023
1 parent 14e5f64 commit fa75693
Show file tree
Hide file tree
Showing 15 changed files with 147 additions and 26 deletions.
121 changes: 121 additions & 0 deletions bin/packages/check-build-type-declaration-files.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
/**
* This script verifies the published index.d.ts file for every package which both
* builds types and also sets checkJs to false in its tsconfig.json. (This scenario
* can cause unchecked errors in JS files to be included in the compiled types.)
*
* We do so by running `tsc --noEmit` on the $package/build-types/index.d.ts file.
* This also verifies everything index.d.ts references, so it checks the entire
* public api of the type declarations for that package.
*
* @see https://github.com/WordPress/gutenberg/pull/49650 for more discussion.
*/

/**
* External dependencies
*/
const fs = require( 'fs' ).promises;
const path = require( 'path' );
const { exec } = require( 'child_process' );
const chalk = require( 'chalk' );

/**
* Returns whether a package needs its compiled types to be double-checked. This
* needs to happen when both of these are true:
* 1. The package compiles types. (It has a tsconfig file.)
* 2. The tsconfig sets checkJs to false.
*
* NOTE: In the future, if we run into issues parsing JSON, we should migrate to
* a proper json5 parser, such as the json5 npm package. The current regex just
* handles comments, which at the time is the only thing we use from JSON5.
*
* @param {string} packagePath Path to the package.
* @return {boolean} whether or not the package checksJs.
*/
async function packageNeedsExtraCheck( packagePath ) {
const configPath = path.join( packagePath, 'tsconfig.json' );

try {
const tsconfigRaw = await fs.readFile( configPath, 'utf-8' );
// Removes comments from the JSON5 string to convert it to plain JSON.
const jsonString = tsconfigRaw.replace( /\s+\/\/.*$/gm, '' );
const config = JSON.parse( jsonString );

// If checkJs both exists and is false, then we need the extra check.
return config.compilerOptions?.checkJs === false;
} catch ( e ) {
if ( e.code !== 'ENOENT' ) {
throw e;
}

// No tsconfig means no checkJs
return false;
}
}

// Returns the path to the build-types declaration file for a package if it exists.
// Throws an error and exits the script otherwise.
async function getDecFile( packagePath ) {
const decFile = path.join( packagePath, 'build-types', 'index.d.ts' );
try {
await fs.access( decFile );
return decFile;
} catch ( err ) {
console.error(
`Cannot access this declaration file. You may need to run tsc again: ${ decFile }`
);
process.exit( 1 );
}
}

async function typecheckDeclarations( file ) {
return new Promise( ( resolve, reject ) => {
exec( `npx tsc --noEmit ${ file }`, ( error, stdout, stderr ) => {
if ( error ) {
reject( { file, error, stderr, stdout } );
} else {
resolve( { file, stdout } );
}
} );
} );
}

async function checkUnverifiedDeclarationFiles() {
const packageDir = path.resolve( 'packages' );
const packageDirs = (
await fs.readdir( packageDir, { withFileTypes: true } )
)
.filter( ( dirent ) => dirent.isDirectory() )
.map( ( dirent ) => path.join( packageDir, dirent.name ) );

// Finds the compiled type declarations for each package which both checks
// types and has checkJs disabled.
const declarations = (
await Promise.all(
packageDirs.map( async ( pkg ) =>
( await packageNeedsExtraCheck( pkg ) )
? getDecFile( pkg )
: null
)
)
).filter( Boolean );

const tscResults = await Promise.allSettled(
declarations.map( typecheckDeclarations )
);

tscResults.forEach( ( { status, reason } ) => {
if ( status !== 'fulfilled' ) {
console.error(
chalk.red(
`Incorrect published types for ${ reason.file }:\n`
),
reason.stdout
);
}
} );

if ( tscResults.some( ( { status } ) => status !== 'fulfilled' ) ) {
process.exit( 1 );
}
}
checkUnverifiedDeclarationFiles();
26 changes: 13 additions & 13 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@
"scripts": {
"build": "npm run build:packages && wp-scripts build",
"build:analyze-bundles": "npm run build -- --webpack-bundle-analyzer",
"build:package-types": "node ./bin/packages/validate-typescript-version.js && tsc --build",
"build:package-types": "node ./bin/packages/validate-typescript-version.js && tsc --build && node ./bin/packages/check-build-type-declaration-files.js",
"prebuild:packages": "npm run clean:packages && lerna run build",
"build:packages": "npm run build:package-types && node ./bin/packages/build.js",
"build:plugin-zip": "bash ./bin/build-plugin-zip.sh",
Expand Down
2 changes: 1 addition & 1 deletion packages/annotations/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
"@wordpress/hooks": "file:../hooks",
"@wordpress/i18n": "file:../i18n",
"@wordpress/rich-text": "file:../rich-text",
"rememo": "^4.0.0",
"rememo": "^4.0.2",
"uuid": "^8.3.0"
},
"publishConfig": {
Expand Down
2 changes: 1 addition & 1 deletion packages/block-editor/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@
"lodash": "^4.17.21",
"react-autosize-textarea": "^7.1.0",
"react-easy-crop": "^4.5.1",
"rememo": "^4.0.0",
"rememo": "^4.0.2",
"remove-accents": "^0.4.2",
"traverse": "^0.6.6"
},
Expand Down
2 changes: 1 addition & 1 deletion packages/blocks/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
"is-plain-object": "^5.0.0",
"lodash": "^4.17.21",
"memize": "^1.1.0",
"rememo": "^4.0.0",
"rememo": "^4.0.2",
"remove-accents": "^0.4.2",
"showdown": "^1.9.1",
"simple-html-tokenizer": "^0.5.7",
Expand Down
2 changes: 1 addition & 1 deletion packages/commands/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
"@wordpress/keyboard-shortcuts": "file:../keyboard-shortcuts",
"@wordpress/private-apis": "file:../private-apis",
"cmdk": "^0.2.0",
"rememo": "^4.0.0"
"rememo": "^4.0.2"
},
"peerDependencies": {
"react": "^18.0.0"
Expand Down
2 changes: 1 addition & 1 deletion packages/core-data/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
"equivalent-key-map": "^0.2.2",
"fast-deep-equal": "^3.1.3",
"memize": "^1.1.0",
"rememo": "^4.0.0",
"rememo": "^4.0.2",
"uuid": "^8.3.0"
},
"peerDependencies": {
Expand Down
2 changes: 1 addition & 1 deletion packages/edit-post/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@
"@wordpress/widgets": "file:../widgets",
"classnames": "^2.3.1",
"memize": "^1.1.0",
"rememo": "^4.0.0"
"rememo": "^4.0.2"
},
"peerDependencies": {
"react": "^18.0.0",
Expand Down
2 changes: 1 addition & 1 deletion packages/edit-site/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@
"lodash": "^4.17.21",
"memize": "^1.1.0",
"react-autosize-textarea": "^7.1.0",
"rememo": "^4.0.0"
"rememo": "^4.0.2"
},
"peerDependencies": {
"react": "^18.0.0",
Expand Down
2 changes: 1 addition & 1 deletion packages/editor/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@
"lodash": "^4.17.21",
"memize": "^1.1.0",
"react-autosize-textarea": "^7.1.0",
"rememo": "^4.0.0",
"rememo": "^4.0.2",
"remove-accents": "^0.4.2"
},
"peerDependencies": {
Expand Down
2 changes: 1 addition & 1 deletion packages/keyboard-shortcuts/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
"@wordpress/data": "file:../data",
"@wordpress/element": "file:../element",
"@wordpress/keycodes": "file:../keycodes",
"rememo": "^4.0.0"
"rememo": "^4.0.2"
},
"peerDependencies": {
"react": "^18.0.0"
Expand Down
2 changes: 1 addition & 1 deletion packages/react-native-editor/jest_ui.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ module.exports = {
platforms: [ 'android', 'ios', 'native' ],
},
transform: {
'^.+\\.(js|ts|tsx)$': 'babel-jest',
'^.+\\.(js|ts|tsx|cjs)$': 'babel-jest',
},
setupFilesAfterEnv: [ './jest_ui_setup_after_env.js' ],
testEnvironment: './jest_ui_test_environment.js',
Expand Down
2 changes: 1 addition & 1 deletion packages/react-native-editor/metro.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const packageNames = fs.readdirSync( PACKAGES_DIR ).filter( ( file ) => {
module.exports = {
watchFolders: [ path.resolve( __dirname, '../..' ) ],
resolver: {
sourceExts: [ 'js', 'json', 'scss', 'sass', 'ts', 'tsx' ],
sourceExts: [ 'js', 'cjs', 'json', 'scss', 'sass', 'ts', 'tsx' ],
platforms: [ 'native', 'android', 'ios' ],
},
transformer: {
Expand Down
2 changes: 1 addition & 1 deletion packages/rich-text/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
"@wordpress/i18n": "file:../i18n",
"@wordpress/keycodes": "file:../keycodes",
"memize": "^1.1.0",
"rememo": "^4.0.0"
"rememo": "^4.0.2"
},
"peerDependencies": {
"react": "^18.0.0"
Expand Down

0 comments on commit fa75693

Please sign in to comment.