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

Add path alias to nest new #13294

Closed
1 task done
GustavoOS opened this issue Mar 5, 2024 · 10 comments
Closed
1 task done

Add path alias to nest new #13294

GustavoOS opened this issue Mar 5, 2024 · 10 comments
Labels
needs triage This issue has not been looked into type: enhancement 🐺

Comments

@GustavoOS
Copy link

Is there an existing issue that is already proposing this?

  • I have searched the existing issues

Is your feature request related to a problem? Please describe it

I have an issue when I try to setup path alias, sometimes causing basic modules not to be found

Describe the solution you'd like

Adding a possibility to configure path alias in the CLI, similar to create-next-app
image

Teachability, documentation, adoption, migration strategy

When creating a new project, ask for the possibility to use import alias.

What is the motivation / use case for changing the behavior?

Avoid using long relative paths on import statements.

@GustavoOS GustavoOS added needs triage This issue has not been looked into type: enhancement 🐺 labels Mar 5, 2024
@jmcdo29
Copy link
Member

jmcdo29 commented Mar 5, 2024

I'm personally against using path aliases within a single application project. Typescript supports it for the sake of libraries, and Nest integrates with tsconfig-paths to keep people from complaining to us that path aliases don't work, but I don't think we should be supporting this pattern by default. If you were to just run tsc and node dist/main, it actually could break how things run, and the reason isn't evident to a newcomer.

@GustavoOS
Copy link
Author

The goal is not to have it by default, instead, opt-in without the hassle of doing and breaking the project.

@jmcdo29
Copy link
Member

jmcdo29 commented Mar 5, 2024

By having it as an option, in my opinion, we would be encouraging bad development practices, which I don't believe fall inline with the desires of the framework. I'll let @kamilmysliwiec have the final say, but that's my two cents

@GustavoOS
Copy link
Author

By having it as an option, in my opinion, we would be encouraging bad development practices, which I don't believe fall inline with the desires of the framework. I'll let @kamilmysliwiec have the final say, but that's my two cents

Why would this be a bad development practice?

See, when you create a new NestJS project today, on app.e2e-spec.ts we get the following import statement:
import { AppModule } from './../src/app.module'

With path alias, this would become something like:
import { AppModule } from '@/app.module'

Much cleaner.

@micalevisk
Copy link
Member

micalevisk commented Mar 5, 2024

but now you're kinda mixing testing config with app config. But yeah, it's cleaner for sure

To me, ideally, we need a tsconfig file for test dir and another for src, which is not the case for the current version of the typescript starter

@jmcdo29
Copy link
Member

jmcdo29 commented Mar 5, 2024

Why would this be a bad development practice?

Again, Typescript allows for aliases for the sake of libraries. Typescript's golden rule is to not change the import statements during compilation (save for import to require for CJS). Nest makes this work by integrating tsconfig-paths, but that's mainly because there were way too many "it doesn't work" issues where people didn't know that Typescript didn't actually behave this way. So if Typescript doesn't do this by default, I think we shouldn't encourage it either. Again, my two cents, and I'll leave the final decision to Kamil, but it's certainly something I think we should keep in mind.


Also, your first import statement has an extra ./ that could be removed. Not too much cleaner, but a bit

@GustavoOS
Copy link
Author

but now you're kinda mixing testing config with app config. But yeah, it's cleaner for sure

To me, ideally, we need a tsconfig file for test dir and another for src, which is not the case for the current version of the typescript starter

For the way NestJS organizes it's folders, maybe 2 aliases would be better, one for src and another for test. The tsconfig.json would become like:

{
    "compilerOptions": {
//  other options
        "paths": {
             "@/*": ["src/*"],
              "#/*": ["test/*"],
        }
    }
}

As a consequence, a ts file created under test folder could have it's import statement like
import { getTestApp } from '#/util';

@micalevisk
Copy link
Member

micalevisk commented Mar 5, 2024

that's even worse to me because, unlike Next.js, nestjs doesn't need to tell us on how our path aliases should look like. Perhaps we just need to make a recipe in the docs site on how to use path aliases

@GustavoOS
Copy link
Author

A recipe would be a good fit, I think

@kamilmysliwiec
Copy link
Member

This is a TypeScript compiler's feature. I don't see a reason why we should be duplicating docs and creating a recipe on something that Nest didn't introduce in the first place

#13294 (comment) couldn't agree more

@nestjs nestjs locked and limited conversation to collaborators Mar 6, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs triage This issue has not been looked into type: enhancement 🐺
Projects
None yet
Development

No branches or pull requests

4 participants