alveifbklsiu259 commented on code in PR #32261:
URL: https://github.com/apache/superset/pull/32261#discussion_r1957032311
##########
.pre-commit-config.yaml:
##########
@@ -68,6 +68,39 @@ repos:
language: system
pass_filenames: true
files: \.(js|jsx|ts|tsx)$
+ - repo: local
+ hooks:
+ - id: type-checking-frontend
+ name: Type-Checking (Frontend)
+ entry: ./scripts/check-type.js
Review Comment:
> From my understanding this will check all the files, not just the ones
touched in the commit is that right?
It seems that we don't have to explicitly pass `$@`, pre-commit will only
passed the touched files:
```js
// check-type.js
void (async () => {
const args = process.argv.slice(2);
console.log(args)
exit(1)
```
> is it all super fast?
I wouldn't say it is **super fast**, but it is faster than checking all
files.
> Key here is to somehow pass the list of files touched in the commit
(provided by pre-commit), and only run the checks on those files.
This is exactly what this pre-commit hook does.
> ... may have to go and import all the related files referenced in that
seed file, and could end up taking quite some time if you end up covering
files-that-import-files-that-import-other-files
This is still true for this hook. For example:
```ts
// superset-frontend/src/components/AsyncAceEditor/index.tsx
import AsyncEsmComponent, {
PlaceholderProps,
} from 'src/components/AsyncEsmComponent';
```
If `superset-frontend/src/components/AsyncAceEditor/index.tsx` is the only
file in the staging area, running this hook will also check
`src/components/AsyncEsmComponent/index.tsx` and the files it imports and so on.
> Wondering if it would make sense to only do it on
superset-frontend/**/js-and-ts-files to keep pre-commit.yml a bit simpler.
Sure, as long as we only want to type-check the files under
`superset-frontend`, but we still need to exclude
`superset-frontend/cypress-base/`, since `superset-frontend/cypress-base/` uses
a different `tsconfig.json` and it contains a conflicting declaration file.
i.e. `superset-frontend/cypress-base/cypress/support/index.d.ts`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org
