Skip to content
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

TypeScript path aliasses #865

Closed
1 of 4 tasks
Pidz-b opened this issue Oct 7, 2019 · 22 comments · Fixed by #3662
Closed
1 of 4 tasks

TypeScript path aliasses #865

Pidz-b opened this issue Oct 7, 2019 · 22 comments · Fixed by #3662
Labels
effort/large Large work item – several weeks of effort feature-request A feature should be added or improved. module/tools Issues related to the JSII toolkit p2 style Issues related to coding style

Comments

@Pidz-b
Copy link

Pidz-b commented Oct 7, 2019

🚀 Feature Request

Affected Languages

  • TypeScript or Javascript
  • Python
  • Java
  • .NET (C#, F#, ...)

General Information

  • JSII Version: AWS CDK 1.11.0 (build 4ed4d96)
  • Platform: Linux 5.2.11-1-MANJARO

What is the problem?

When running cdk deploy or cdk synth, a path alias defined in in tsconfig.json is not resolved to the correct path.

I have all my CDK code in a folder named <project root>/infra/
In one of my files I import a custom file that abstracts our naming:

import { NameGen } from '@infra/lib/NameGen';

The tsconfig.json paths look like:

{
  "compilerOptions": {
    "paths": {
      "@infra/*": ["infra/*"],
      ....
}

When running CDK, the following error is thrown: Cannot find module '@infra/lib/NameGen'


Apparently JSII does not resolve the path aliasing of the tsconfig correctly?

@Pidz-b Pidz-b added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Oct 7, 2019
@RomainMuller
Copy link
Contributor

JSII does not allow you to override it's generated tsconfig.json; and thus currently setting paths is not supported.

Can you please laborite on what the use-case for path aliases is, so we can determine whether this is a feature that we should support?

@RomainMuller RomainMuller added feature-request A feature should be added or improved. and removed bug This issue is a bug. labels Oct 7, 2019
@Pidz-b
Copy link
Author

Pidz-b commented Oct 7, 2019

The use case is pretty code.

The only solution for this is to write the relative path out in full:

import { NameGen } from '../../../../infra/lib/NameGen';
import { MatchingStack } from '../../../../infra/MatchingStack';

In the code I generate for AWS lambda, I use path aliases to get rid of relative-path-hell:

require('@lib/bootstrap');
import FreelancerActivatedEventBody from '@event-handlers/FreelancerActivated/FreelancerActivatedEventBody';
import { SnsEventHandler } from '@lib/SnsEventHandler';

It makes the imports a lot more humanreadable and cleans up the code.


I hope clean code is a good enough reason to implement is, as clean code is a high priority for readability and maintainability IMO

@eladb
Copy link
Contributor

eladb commented Oct 7, 2019

We should stop including tsconfig.json files in our modules. It's not a good practice. I remember @rix0rrr said there was some reason related to doc generation??

@Pidz-b
Copy link
Author

Pidz-b commented Oct 7, 2019

We should stop including tsconfig.json files in our modules. It's not a good practice. I remember @rix0rrr said there was some reason related to doc generation??

Is this related to my issue? Or related to the JSII compiler?

@SomayaB SomayaB removed the needs-triage This issue or PR still needs to be triaged. label Nov 18, 2019
@SomayaB SomayaB added style Issues related to coding style module/tools Issues related to the JSII toolkit labels Jan 7, 2020
@MrArnoldPalmer MrArnoldPalmer added the effort/large Large work item – several weeks of effort label Feb 12, 2020
@Luca-Terrazzan
Copy link

Luca-Terrazzan commented Apr 10, 2020

This issue is still present in cdk as well (it uses JSII). In my case I would love to use ts paths in order to provide locators for my lambdas code.

@RomainMuller
Copy link
Contributor

I'm contemplating whether we should allow user-controlled tsconfig.json (and restrict certain compiler options to maintain the needed compatibility -- for example, target <= "ES2018", module == "CommonJS", etc...). This would allow us not to have to worry about supporting all variants of inputs that tsc supports that users would want...

What do you guys think about this idea?

@MrArnoldPalmer
Copy link
Contributor

It seems simple enough to parse a user provided tsconfig.json if present and merge it with the required/default tsconfig for jsii. We should just warn when a user is including some config that is incompatible with JSII and we are overriding it. Or crash with non-zero.

@JeremyJonas
Copy link

This also effects lerna (yarn workspace) based projects, as workspace packages can not be resolved to src instead of "main" path. Which means workspace packages used in cdk must be built first, rather than using source typescript files. Very annoying.

{
  "compilerOptions": {
    ...
    "baseUrl": "../",
    "paths": {
      "*": ["packages/*/src", "*", "@types/*"]
    }
  },
  "exclude": ["node_modules", "cdk.out", ".out"]
}

@RomainMuller
Copy link
Contributor

@JeremyJonas - I'm not sure I understand how your project is set up. The aws/aws-cdk repository uses lerna with yarn and the workflow feels pretty natural to me.

Would you be able to explain your project structure in more tail, or better yet, share a pointer to an example repository that uses it?

@JeremyJonas
Copy link

@RomainMuller I’ll try to send example code next week when get back to this.

But basically I have a /packages/... with typescript packages. Those packages use the above tsconfig to reference each other. Using the paths they reference the /src code rather than /dist declaration files for type, which works great. These are all included in lerna/yarn workspaces.

I then have a /infra dir at root that is also included as lerna/yarn workspace. It has similar tsconfig file that includes paths but to the ../packages relative to that dir.

But since cdk tsconfig doesn’t support alias paths it doesn’t work. But it does resolve to node_modules symlink of those /packages.

But because node_modules resolution is based on “main” which is /dist/index.d.ts from complies typescript it requires first building all the packages before /infra cdk package can use them.

Hope that makes sense.

Appreciate you guidance and support!

Will provide more details if needed next week.

@RomainMuller
Copy link
Contributor

RomainMuller commented Jul 9, 2020

Have you tried out the --project-references option of jsii? If you use that jsii will operate in a mode similar to tsc --build.

@JeremyJonas
Copy link

@RomainMuller Do you have a example of that working? Docs are pretty sparse on that, only thing I found was aws/aws-cdk#1466. Just tried to setup tsconfig references and same issue, need to build first.

If that is like tsconfig "references", does it have the same limitation of having to build referenced packages before they can be references? https://www.typescriptlang.org/docs/handbook/project-references.html#caveats-for-project-references

If that is the case, basically same issue I have. Currently with my lerna/yarn project everything is in root node_modules, so after building it just works. But with aliases no need to build, can reference src directly :)

@bobbyhadz
Copy link

This feature would definitely be a quality of life improvement, to avoid doing:

import {x} from '../../../../folder/file'

And to be consistent with frontend imports, I would assume most people don't you relative imports, when working in typescript.

@hirenumradia
Copy link

I ran into this issue today, Its a shame I have to use relative imports in my microservices. The way my project is structured is as follows.

Tooling

  1. Yarn V2
  2. Workspaces
  3. Lerna

Structure

  • "Backend package" creates a CDK stack
  • Each microservice is a separate package included in the "Backend package" as a NestedStack

Problem

  • Path imports within a microservice are not resolved when running "cdk bootstrap" on the Backend package.

Current Workaround

  • I am currently using relative imports within the microservices. This makes me sad :(

@MrArnoldPalmer MrArnoldPalmer removed their assignment Jun 28, 2021
@bigo104
Copy link

bigo104 commented Jul 26, 2021

Is this issue still a thing?

@skymandr
Copy link

Is this issue still a thing?

I just ran into it I think, so seemingly, yes. :-/

@moltar
Copy link

moltar commented Sep 24, 2021

Not an issue in our project anymore.

@55Cancri
Copy link

@moltar Well how did you solve it then? I am ASTONISHED cdk doesn't support path aliases, which essentially breaks my entire backend workflow. @RomainMuller please add this feature to cdk.

@moltar
Copy link

moltar commented Oct 24, 2021

@moltar Well how did you solve it then? I am ASTONISHED cdk doesn't support path aliases, which essentially breaks my entire backend workflow. @RomainMuller please add this feature to cdk.

Here's a demo, hope that helps: https://github.com/moltar/cdk-ts-path-mapping

@axelra82
Copy link

axelra82 commented Jul 18, 2022

Kind of surprising that this is still an issue 🤔

As @Pidz-b said

I hope clean code is a good enough reason to implement is, as clean code is a high priority for readability and maintainability IMO

Agreed 100%.

And

In the code I generate for AWS lambda, I use path aliases to get rid of relative-path-hell

Having to install tsconfig-paths and then adding -r tsconfig-paths/register to app in cdk.json as per c6a4993692e5a74d3884f786d8eb282181f0ed92 works, but seems like something that should just work out of the box.

Relative paths are bad practice.

RomainMuller added a commit that referenced this issue Jul 18, 2022
Allow user control of the `baseUrl` and `paths` compiler options in
`tsconfig.json` via the `jsii.tsc` stanza in `package.json`.

Fixes #865
@mergify mergify bot closed this as completed in #3662 Jul 18, 2022
mergify bot pushed a commit that referenced this issue Jul 18, 2022
Allow user control of the `baseUrl` and `paths` compiler options in
`tsconfig.json` via the `jsii.tsc` stanza in `package.json`.

Fixes #865



---

By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license].

[Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0
@github-actions
Copy link
Contributor

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

@heiba
Copy link

heiba commented Jul 22, 2022

Hello @RomainMuller, thanks for merging this feature as per #3662, this means the next cdk release will include the latest changes in jsii (namely this feature) ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort/large Large work item – several weeks of effort feature-request A feature should be added or improved. module/tools Issues related to the JSII toolkit p2 style Issues related to coding style
Projects
None yet
Development

Successfully merging a pull request may close this issue.