-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
refactor: cleanup cli/js/compiler.ts #3347
Comments
One more thing to keep in mind: ideally we could add more entry points, but that's not possible now without increasing binary size. |
If we're refactoring cli/js/compiler.ts it should be done in way that can share more code with https://github.com/denoland/deno/blob/master/deno_typescript/compiler_main.js (which essentially serves the same purpose) |
Sure, but that means downgrading most of the code to JS with JSDoc, right? |
@bartlomieju be careful, I've got some stuff for the bundling going on in another PR right now... I think it can be rationalised more, I have pulled some stuff out that really doesn't have to deal with the host, but there is part of me that would like us to get some the transpile stuff in (like the userland API access to the compiler (#2927), and potentially this transpile only feature (#3321)) before we really have a grasp of how to structure the compiler. Specifically we wouldn't want to un-DRY anything, but exploding out some of the functionality so it is a bit easier to maintain might make sense. @ry I would like to too... though I would hate to "regress" to JavaScript only... I wonder if we could bootstrap the TypeScript compiler, bring up a minimal program to transpile the host in TypeScript, and possibly also the bundle loader, and then transpile the bundles. |
Thanks for the heads up @kitsonk! In that case I'll hold off until you're done. |
I think regressing to JavaScript isn't such a big deal. The interface to TS is a relatively small bit of code - and it isn't exposed to the users. Because of the bootstrapping problem, I think it's a lot less complex to just write this part in JS. |
Ok, one thought is that we enable checkJs on deno_typescript. |
We should turn checkJs on deno_typescript. At that point we can still have confidence whatever is in the internal bundles is type safe. |
Done in #3442 |
cli/js/compiler.ts
became a little unwieldy and could use some cleaning.I think that we could factor out
Host
,BundlerHost
andCompilerHost
and maybe splitcompiler.ts
into more files; maybejs/compiler.ts
andjs/compiler/host.ts
?I will work on that, posting for tracking
The text was updated successfully, but these errors were encountered: