Lazy require 'typescript' module. #72
Conversation
indexzero
force-pushed
the
lazy-typescript
branch
from
0d43817 to
a4f3bbd
Compare
Codecov Report
@@ Coverage Diff @@
## master #72 +/- ##
=========================================
- Coverage 68.61% 68.6% -0.02%
=========================================
Files 7 7
Lines 427 430 +3
Branches 120 120
=========================================
+ Hits 293 295 +2
- Misses 85 86 +1
Partials 49 49
Continue to review full report at Codecov.
|
src/ts-node.ts
Outdated
| let parseConfigFileTextToJson | ||
| try { | ||
| // Lazy-require 'typescript' to support JavaScript @oclif projects. | ||
| ({parseConfigFileTextToJson} = require('typescript')) |
There was a problem hiding this comment.
This should probably just be done with dynamic import behaviors, something like the following:
async function loadTSConfig(root: string): TSConfig | undefined {
const tsconfigPath = path.join(root, 'tsconfig.json')
if (fs.existsSync(tsconfigPath)) {
const { parseConfigFileTextToJson } = await import('typescript')
|
|
||
| import Debug from './debug' | ||
| const debug = Debug() | ||
|
|
||
| let typescript: typeof import('typescript') |
There was a problem hiding this comment.
Since I am a dumb dummy, what is the purpose of dynamically importing typescript?
Couldn't you write this without the dynamic import? Also I haven't seen the let typescript: typeof import('typescript') syntax before. What is let typescript: doing?
let typescript;
try {
typescript = require('typescript')
} catch (ex) {
debug('Cannot find typescript', ex)
}
There was a problem hiding this comment.
You sir, are thinking like JavaScript! This was my original implementation, but TypeScript got very cranky with me since the type for let typescript could not be inferred automagically.
There was a problem hiding this comment.
Thank you for the explanation!
I am still not Typescript convinced, and I come from C# and Java background. I just haven't come across issues that I can see typescript solving that proper testing, prs, lint, and other process can't solve. I also can't see typescript solving the problems of those other areas, so they would still be required.
There was a problem hiding this comment.
I am a JavaScript person as well, but hey – open source means exploring new things 🤷♂️
There was a problem hiding this comment.
Thinking further about your explanation, what does the type actually end up being from this:
let typescript: typeof import('typescript')
Wouldn't it be a promise? So it's saying that typescript is typeof promise, but it's not really a promise? Wouldn't it just be simpler to set the type to any or object or whatever type typescript would be?
There was a problem hiding this comment.
there are no types when the code is running, typescript types are only used at compile-time
There was a problem hiding this comment.
So I guess it's working because since typescript has to run to compile this source code, typescript is available at compile time in this scenario. So it just works. I wonder if this would work if the dependency was something other than typescript.
PS. Sorry for the chatty conversation, I fully respect your inboxes!
There was a problem hiding this comment.
@blackdynamo - I agree with you. While it works, I think it's technically incorrect that typescript is being required at runtime, but not listed as a runtime dependency.
There was a problem hiding this comment.
Okay, storytime. Back in the day there was a dream – a dream called optionalDependencies.
The current behavior of optionalDependencies is just a tiny fraction of what was discussed when it was being planned ... at least 6 years ago.
Behavior that would have covered this scenario: opt-in dependencies. What I mean by that is typescript is not actually a run-time dependency. It is a "nice to have" that only attempts to parse tsconfig.json file when present.
There are a lot of grey areas like this in production scenarios for package management. Lazy require is a well known technique to handle them which imho is appropriate here.
There was a problem hiding this comment.
TIL! Thanks for the further explanation 👍
This is blocking. Currently any fresh install of a pure JavaScript
@oclifproject will die with:Debugging aside, this is due to the fact that these JavaScript projects have no need to have
typescriptin thedependenciesordevDependenciesof theirpackage.json.Solution: since
src/ts-node.jsappears to exist to load TypeScript definitions for plugins and other modules consumed by a given CLI project using@oclifwe only requiretypescriptwhen we have confirmed that atsconfig.jsonexists within that directory.Not sure the right approach to test this, but hopefully we can land a hotfix and then discuss testability / debuggability separately.