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

Path aliases in create-t3-app code #206

Closed
ochicf opened this issue Jul 17, 2022 · 10 comments · Fixed by #247
Closed

Path aliases in create-t3-app code #206

ochicf opened this issue Jul 17, 2022 · 10 comments · Fixed by #247
Labels
🔰 good first issue Good for newcomers 🌟 enhancement New feature or request 👨‍👦‍👦free for all Anyone is free to take on this issue and file a PR

Comments

@ochicf
Copy link
Contributor

ochicf commented Jul 17, 2022

Is your feature request related to a problem? Please describe.
We decided that the scaffolded app will not have path aliases here: #85

But it was raised that maybe for the create-t3-app code would be worth having a new discussion, as this would only affect contributors and they were not the target of the decisions taken in the original issue.

Describe the solution you'd like
Define path aliases in create-t3-app so contributors have an easier and consistent way of importing code. In this issue we can propose/discuss what is the path aliases that we want to have, then decide and create a MR from it.

Describe alternatives you've considered
Leave imports as they are.

@ochicf
Copy link
Contributor Author

ochicf commented Jul 17, 2022

My proposal for code in src folder:

  • symbol: ~
    • (+) not confused with the @ of namespaced packages
    • (+) uses the already common pattern of describing "home" in unix systems
    • (-) not sure if it's a char commonly accessible in all keyboard layouts? (in the spanish layout it's easy to type)
  • targets: ~/* to src/*. This would mean that ~/helpers point to src/helpers, ~/utils to src/utils, etc.
    • (+) behaves as a folder
    • (+) no need to set new aliases if new folders/files in src are created
    • (-) would allow to import some files that shouldn't be imported such as src/index.ts

I would like to raise an extra point aswell: in #116 (comment) I've defined a typescript submodule, and I would like to have path aliases there aswell. We should avoid clashes with the ones defined above so I would propose to use the ~template/* approach (where template would be the name of the sub-package, in case we add new ones).

  • (+) uses same symbol
  • (-) not fully consistent with how the ~ behaves for src as described above
  • (-) we would have to define a new alias for each new sub-package (which I don't think it will happen that often though)
  • (-) in comparison to ~* to src/*, we may have weird cases with the symbol ~ and leading vocals such as a and o where typing ~ and that vocal would yield ã and õ, having to type a space in the middle to prevent this.

Some examples with all of the above:

// importing from src
import { PKG_ROOT } from "~/consts.ts";
import { buildPkgInstallerMap } from "~/installers/index.ts";
import { getUserPkgManager } from "~/utils/getUserPkgManager.js";


// importing from a subpackage
import { BaseContextTransformOptions } from "~transform/addons/trpc/base-context.js";

@alonzuman
Copy link

looks awesome thats literally what i was about to open a PR for

@t3dotgg
Copy link
Member

t3dotgg commented Jul 18, 2022

Path aliases make moving to monorepos quite painful and are generally very opinionated. I think we decided awhile back to not do this for those reasons

@nexxeln
Copy link
Member

nexxeln commented Jul 18, 2022

Path aliases make moving to monorepos quite painful and are generally very opinionated. I think we decided awhile back to not do this for those reasons

They're talking about path aliases in the CLI code, not the template.

@juliusmarminge
Copy link
Member

Im down for including it in the cli code

@alonzuman
Copy link

Path aliases make moving to monorepos quite painful and are generally very opinionated. I think we decided awhile back to not do this for those reasons

I kind of feel like path aliases are the only way that allows the company I work for to actually move to a monorepo -

We had imports like ../../... x 8 (poor programming I know) and by refactoring to absolute imports, we could move stuff around without being afraid it will break stuff and then we could move to a monorepo when everything is organized, for example:

'src/components/FilePickers/Unsplash/components/UnsplashPicker' is now '/unsplash/components/...' and it imports from '/design-system' (previously ../../../design-system) and finally can be extracted (with turborepo/nx) to packages/unsplash etc'

@alonzuman
Copy link

Path aliases make moving to monorepos quite painful and are generally very opinionated. I think we decided awhile back to not do this for those reasons

I kind of feel like path aliases are the only way that allows the company I work for to actually move to a monorepo -

We had imports like ../../... x 8 (poor programming I know) and by refactoring to absolute imports, we could move stuff around without being afraid it will break stuff and then we could move to a monorepo when everything is organized, for example:

'src/components/FilePickers/Unsplash/components/UnsplashPicker' is now '/unsplash/components/...' and it imports from '/design-system' (previously ../../../design-system) and finally can be extracted (with turborepo/nx) to packages/unsplash etc'

Maybe it should actually be a cli option (but then the next question is what makes something a cli option? Why this and not that etc)

@ochicf
Copy link
Contributor Author

ochicf commented Jul 18, 2022

Sorry if maybe I wasn't clear enough in OP, the intent of this issue is to add path aliases in the code that generates the project, not the code of the project being created.

So to clarify: path aliases resulting from this issue would only be noticeable by contributors of this project.

@juliusmarminge juliusmarminge added 🌟 enhancement New feature or request 💬 discussion Issue needs further discussion before decision 🔰 good first issue Good for newcomers 👨‍👦‍👦free for all Anyone is free to take on this issue and file a PR and removed 💬 discussion Issue needs further discussion before decision labels Jul 19, 2022
@abui-am
Copy link
Contributor

abui-am commented Jul 24, 2022

Great idea, but should we refactor all of cli code using path alias after adding the config?
or should we leave it as it should be?

@juliusmarminge
Copy link
Member

Great idea, but should we refactor all of cli code using path alias after adding the config?

or should we leave it as it should be?

Ideally refactor everything too

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔰 good first issue Good for newcomers 🌟 enhancement New feature or request 👨‍👦‍👦free for all Anyone is free to take on this issue and file a PR
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants