-
Couldn't load subscription status.
- Fork 58
feature: add support for single file type checking #942
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
base: main
Are you sure you want to change the base?
feature: add support for single file type checking #942
Conversation
|
|
||
| #### Single File Checking | ||
|
|
||
| Glint supports checking individual files or a specific set of files instead of your entire project. This can be useful for faster feedback during development or when working with large codebases. |
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.
Do you have numbers on the size of your project and how much of a difference this made?
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.
Yes, > 5000 files.
Running the glint command on all files can take up to 60 seconds (when used with lint-staged). Since we're experiencing issues with the Glint extension frequently crashing, broken code sometimes gets pushed upstream, and TypeScript errors are only caught later when the full command is executed in CI.
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.
Lint staged has a glint plugin? Do you have to use that?
Also, have you tried the v2 alphas and prererease in vscode?
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.
Lint staged has a glint plugin? Do you have to use that?
No need for specific plugins, you can use it like this.
{
"lint-staged": {
"*.{gts,ts}": [
"glint --noEmit"
]
}
}
I wouldn't say we have to use it that way, but it would be super convenient if, along with other linters, it could run glint (lint) on the changed files at commit time together with stylelint, for example, and the others...
Also, have you tried the v2 alphas and prererease in vscode?
I don't think so...
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.
it could run glint (lint) on the changed files at commit time together with stylelint, for example, and the others...
For sure, but typescript doesn't work this way, it has to resolve the types of everything imported.
But! I will concede if with this branch, the outputs are significantly different:
# for reference
glint --version
glint -v
# measure
time glint
time glint ./app/templates/application.gts
time glint ./app/components/something.gtsIf you could get me these numbers, that'd be a huge help
I don't think so...
You'll want to, as we've frozen glint v1 and are only working on glint v2 right now. It's already way faster.
(And this pr is targeting v2)
Caveat tho, we dropped support for hbs.
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.
node glint/packages/core/bin/glint.js --version: Version 5.8.2
time node glint/packages/core/bin/glint.js: 19.93s user 2.93s system 173% cpu 13.200 total
time node glint/packages/core/bin/glint.js app/components/feature-badge.gts: 1.58s user 0.19s system 189% cpu 0.934 total
time node glint/packages/core/bin/glint.js app/components/scrollable-page.gts app/components/feature-badge.gts : 1.50s user 0.17s system 188% cpu 0.881 totalIt correctly detects the errors I intentionally introduce, so everything seems to be working fine.
Also, the 19 seconds is because I ran Glint in just one project (we have a monorepo with multiple projects and packages), so it would take even longer otherwise.
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.
That's pretty good results!
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.
Also, a file with multiple imports will resolve and lint all those imported files as well. But even so, it’s still significantly faster than always running the Glint command across the entire codebase.
| } | ||
| }); | ||
|
|
||
| process.argv = tscArgs; |
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.
Argh should be treated as read only.
Mutating argv can break other code that reads argv
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.
I extracted the process.argv mutation into a helper function so we don't pollute global state and the cleanup is guaranteed to happen even if TypeScript throws an error.
| const hasEmitFlag = Boolean( | ||
| compilerOptions.noEmit || |
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.
I don't think we should support emitting at all in this mode.
You'd get a partial project, which, if you're a library, would be bad for your consumers.
Applications don't have any reason to emit at all, iirc
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.
It makes sense to support emit, as long as the default is noEmit, so why not let users decide? Maybe someone just wants to test a single file and see the final JS output for whatever reason.
But whatever you say, I'm more or less fine with either way.
|
looks like ya have some test failures there. edit: oh, I reloaded the page and there were more commits! |
|
Yes, tests passed. There are still some lint warnings left. Should I fix those as well, or can we just ignore them? |
|
They'll need to be fixed. Running prettier on those files should be all you need<3 |
|
@NullVoxPopuli prettier --write done. ✅ |
|
@BojanRatkovic thank you for your contribution, and the perf improvements definitely seem nice. That said I would like to investigate alternatives before bringing in an extension to the It seems that what you're describing would be useful for all
Is it possible for you to build a CLI tool that wraps the underyling |
|
Some related things https://www.npmjs.com/package/tsc-files So that tsc wrapper to do this seems to already exist (just without glint/vue-tsc support) This has been an issue for nearly 8 years |
Previous idea and discussion: #940
This feature adds support for single file type checking in Glint, allowing developers to type-check individual files instead of the entire project. It enables faster feedback during development by running type checks on specific files rather than performing a full project compilation. The implementation includes CLI support for targeting individual files and updates to the configuration system to handle single-file scenarios.