Skott does not read typescript path aliases if entrypoint is not a TS file

[ACHP]

Context

I tried s skott on a React-Native project where almost all files are using Typescript except for the entry file.
And I noticed that all my path aliases were considered as "third party module". Even with the --tsconfig="./tsconfig.json" argument.

How to reproduce

• Init a 100% Typescript project and setup path aliases:
• Run skott, it should works.
• Now update to entrypoint to use .js file extension
• Now path aliases resolution does not work anymore.

That's because if the entry point is not a TS file, the TS configuration is not loaded
See here:

https://github.com/antoine-coulon/skott/blame/d18c1f800276752431d0177597d1fa4f86db5938/packages/skott/src/skott.ts#L494-L502

Suggestion

If the user use tsconfig as argument for the cli, skott should consider that the project is using typescript

[antoine-coulon]

Hello @ACHP, thanks for opening that very well detailed issue!

It's actually an interesting use case, but I'm curious to know why would a JavaScript file be the entrypoint of a TypeScript project in the first place?

Using the tsconfig option as way to settle that the current target is actually TypeScript would be I think a bit cumbersome because internally the tsconfig option is always tsconfig.json by default so we wouldn't easily know if tsconfig.json comes from user input or not.

Maybe an even more straightforward solution would be to try to build path aliases all the time as skott might support #78 Node.js path aliases (https://nodejs.org/api/packages.html#subpath-imports) as well. I'm not really a fan of doing this type of thing but actually with all the mixes that can happen in a JS/TS project with allowJs and all the different alias configs that can be setup I think that determining alias strategy based on the entrypoint would be a mistake.

[ACHP]

why would a JavaScript file be the entrypoint of a TypeScript project in the first place?

Because we can … 😆 More seriously, I don't think that this is something a user would like to do on purpose, but it can happen with allowJS
For my personal use case, the reasons are:

• The project was migrated from javascript to typescript and the migration is not 100% finished yet
• The entry point does not contains much logic, so keeping it in JS has never been a problem
• Changing the entry point extension might requires to change other project config that would reference this entry point.

I don't think this will happens really often, but when it will, user will have to debug skott source code only to find out they need a typescript entry point

Using the tsconfig option as way to settle that the current target is actually TypeScript would be I think a bit cumbersome because internally the tsconfig option is always tsconfig.json by default so we wouldn't easily know if tsconfig.json comes from user input or not.

Yes, you're right. I tried to implement a fix yesterday after opening the issue, and as you mention tsconfig is always declared so we can't use this approach.

Maybe an even more straightforward solution would be to try to build path aliases all the time as skott might support #78 Node.js path aliases

I think there are 2 distinct issue here:

• How to check that a project is using typescript (and maybe load typescript path aliases)
• And what kind of path aliases can be specified ( jsconfig, webpack aliases, babel aliases, package.json aliases, typescript aliases etc ...).

I think we first need to correctly load config that skott support ( for now Typescript only ), then, think of a strategy to "merge" or select one aliases config over another.

Something like

private async buildFromEntrypoint(entrypoint: string): Promise<void> {

    if (isTypeScriptProject()) {
      this.#workspaceConfiguration.typescript = await buildTSConfig(/*...*/);
    }
    // later
    if (isBabelProject()) {
      this.#workspaceConfiguration.babel = await buildBabelConfig(/*...*/);
    }
    // later
    if (hasJSConfig()) {
      this.#workspaceConfiguration.jsonConfig = await buildJSConfig(/*...*/);
    }
    this.#workspaceConfiguration.pathAliases = readPathAliasesFrom([this.#workspaceConfiguration.typescript, this.#workspaceConfiguration.babel, this.#workspaceConfiguration.jsonConfig]);
  /*...*/
}

This is, IMO, one possible way to do this, if you plan to add multiple pathAliases resolution strategy.

For now if you keep typescript only you can just keep your actual code and replace isTypescriptModule(entryPoint) by isTypescriptProject().

isTypescriptProject() can simply try to resolve the config.tsConfig variable, if the tsconfig file exist, load the config, otherwise skip it

function isTypeScriptProject(){
  // naïve "generic" approach
  return fs.existsSync(config.tsConfig);
}

private async buildFromEntrypoint(entrypoint: string): Promise<void> {

    if (isTypeScriptProject()) {
      this.#workspaceConfiguration.typescript = await buildTSConfig(/*...*/);
    }
   
  /*...*/
}

[antoine-coulon]

Because we can … 😆 More seriously, I don't think that this is something a user would like to do on purpose, but it can happen with allowJS [...]

Yeah you can do many things, even executing SQL inside CSS, that ecosystem is burning my brain sometimes 😆

By the way I was not judging in any kind of way there, just curiously wondering what was the setup because I tried to anticipate as much scenarios as possible but this missed this one.

Anyway I think this type of use case and even the possibility of turning on allowJs as previously mentioned is enough to confirm that whatever decision skott takes, it should not be based upon the entrypoint.

I think there are 2 distinct issue here:

How to check that a project is using typescript (and maybe load typescript path aliases)
And what kind of path aliases can be specified ( jsconfig, webpack aliases, babel aliases, package.json aliases, typescript aliases etc ...).

Yeah absolutely! We'll definitely need at some point to have this type of strategy to resolve path alias configurations from different sources (tsconfig.json, package.json, vite.config.js...).

For now if you keep typescript only you can just keep your actual code and replace isTypescriptModule(entryPoint) by isTypescriptProject() [...]

Yes you're right, that's pretty much what I had in mind feature-wise :)

Regarding the implementation what I was saying try to build all the time path aliases was mostly equivalent to isTypeScriptProject(), the difference being that building path aliases would silently fail if the current project is not a TypeScript one hence not building path aliases (but providing the same result).

Also, we must be careful semantically because it's hard to settle whether a given project is TypeScript or not based only on tsconfig.json because one could provide TypeScript compiler options via CLI without any tsconfig.json file and it would still be a TypeScript project. Consequently, what I'd suggest for now (taking into account that we only have one path alias strategy to deal with) is to try to build TypeScript path aliases in all scenarios, if there is no tsconfig.json the computation will just silently fail. It's like fire and forget. In that case, we might just need to update the logger error message

skott/packages/skott/src/modules/walkers/ecmascript/typescript/path-alias.ts

Line 172 in d18c1f8

} catch (exception: any) {

to avoid misleading users not using TypeScript or not expecting some random tsconfig.json to be searched.

For the future dealing with multiple strategies, we might just need to remove --tsconfig in favor of a flag that maps to whatever supported file that can declare path aliases (tsconfig, package.json, vite.config.js...), because currently tsconfig is only used to look for path aliases, nothing else. From that unique source of truth we'll then be able to construct alias references and keep most of the current logic the same.

[ACHP]

Totally agree with all of this :)
Just some notes though:

• TS path aliases cannot be set using cli arguments (AFAIK). So, on one hand I agree we can't really base a isTypescriptProject function on that strategy, BUT on the other hand, it would kinda be enough for this usecase (type aliases)
• If you try to build path aliases all the time and decide to fail silently, we might need to differenciate "normal" failure ( no aliases, no tsConfig…), from the "real" failure (bad tsconfig format, bad aliases, skott's own bug etc …)

[antoine-coulon]

Yeah you're right both fit well for now, so let's go on the solution you suggested, I think it's pretty good for the first iteration!

Would be willing to land a PR and contributing on this subject and/or the other issue you opened #83? It's totally up to you, on my side I'm already grateful for the discussion and the opening of the issues, so I could do it myself as well but mainly working on #72 so it could take a bit longer for me to land it :)

[ACHP]

Yes, I'll do that ;)
I'll will probably start with a naïve approach first so we can talk about it in the PR comments (And I'm I'm not familiar with EffectTS for now).

[antoine-coulon]

Glad to hear that, sounds good to me! You can go full TypeScript if you're more comfortable with it, I'm mainly using Effect for dependency injection on skott for now so nothing really fancy. If you ever want to know more though, I authored an introduction about it https://github.com/antoine-coulon/effect-introduction :)

[antoine-coulon]

Should be fixed by #89 released in 0.28.3.
Waiting for @ACHP to check that it works his setup before closing the issue.

[ACHP]

Works great ! 🚀 🚀 🚀

Now I got some mess to clean

[antoine-coulon]

@ACHP nice, glad to see that you made it work! Now I need to speed up the revamp #72 because that visualization is getting ugly and messy for big graphs... To improve the graph visualization I would suggest you to filter a little bit the parts of your project so that you reduce the number of nodes displayed (either via ignorePattern or cwd to select a sub directory). Hopefully the next webapp version will erase all these drawbacks and make the visualization smooth, even for big projects.

[antoine-coulon]

Closing this at it was solved in #89

Thanks again for the nice work @ACHP, appreciate the contributions :)