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

chore(url): add jsdoc typescript checking #17014

Merged
merged 4 commits into from
Aug 28, 2019
Merged
Changes from 1 commit
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
Prev Previous commit
Next Next commit
add typescript checks to lint job
dsifford committed Aug 21, 2019
commit 28d8a0126a8abefa79cacd069186b039426e4f47
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -189,13 +189,14 @@
"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",
"fixtures:regenerate": "npm run fixtures:clean && npm run fixtures:generate",
"lint": "concurrently \"npm run lint-js\" \"npm run lint-pkg-json\" \"npm run lint-css\"",
"lint": "concurrently \"npm run lint-js\" \"npm run lint-pkg-json\" \"npm run lint-css\" \"npm run lint-types\"",
"lint-js": "wp-scripts lint-js",
"lint-js:fix": "npm run lint-js -- --fix",
"lint-php": "docker-compose run --rm composer run-script lint",
"lint-pkg-json": "wp-scripts lint-pkg-json ./packages",
"lint-css": "wp-scripts lint-style '**/*.scss'",
"lint-css:fix": "npm run lint-css -- --fix",
"lint-types": "tsc",
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we install the typescript dev dependency? I think travis is failing because of it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whoops, sorry about that. On it now.

Copy link
Member

Choose a reason for hiding this comment

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

Should we include this command in lint-staged section of this file to have it executed as a pre-commit hook?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure if we can lint only a single file at a time because type compilation generally requires knowledge of the entire project.

Also, the compiler seems to freak out if you feed it files with a .js extension by itself (regardless of whether or not they are allowed)...

$ tsc packages/autop/src/index.js
error TS6054: File 'packages/autop/src/index.js' has unsupported extension. The only supported extensions are '.ts', '.tsx', '.d.ts'.


Found 1 error.

So, in short, we can add it it the lint-staged flow, but it technically won't be able to lint just the staged file(s). Up to you.

Copy link
Member

Choose a reason for hiding this comment

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

We can skip it for now if it produces issues, no worries. As long as Travis is going to catch those violations, we are good 👍

"package-plugin": "./bin/build-plugin-zip.sh",
"pot-to-php": "./bin/pot-to-php.js",
"precommit": "lint-staged",