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

Removed ~/assets alias and baseUrl from tsconfig presets. #7940

Closed
wants to merge 1 commit into from
Closed

Removed ~/assets alias and baseUrl from tsconfig presets. #7940

wants to merge 1 commit into from

Conversation

kanashimia
Copy link
Contributor

@kanashimia kanashimia commented Aug 3, 2023

Changes

Removed ~/assets alias and baseUrl from tsconfig presets.
Instead of predefining ~/assets and baseUrl in our tsconfig presets user should now define them in their own in their tsconfig.json, as predefined aliases were too opinionated and didn't work without modifying tsconfig anyways.

By opinionated i mean that it not only forces a specific location, but also a specific prefix, some users would want to use assets like @assets/blah.png, it just doesn't make much sense to define this in the preset.

Original justification for defining src/assets as a stable directory was in withastro/roadmap#468 (reply in thread)
Justification for defining ~/assets as an alias was in withastro/roadmap#468 (reply in thread)

User has to add baseUrl into their tsconfig for this to work anyway, and this only adds alias for ~/assets and not ~/content or whatever, it just doesn't make much sense, src/assets isn't really any more special than src/content.

Fixes #7470 (you should also read discussion there)

Testing

No tests were added.
tsconfig.json was added at the base dir of all tests that use ~/assets alias, they weren't removed as it may be useful to test if aliases regular work there.
Do you think tests (relevant parts) should be removed instead?

Tested with pnpm --filter astro run test

Docs

Builtin ~/assets alias in the astro assets example should be removed or modified in the docs.

In theory this change can affect users that don't use astro:assets, only if they had baseUrl set in their tsconfig, i doubt it matters.

/cc @withastro/maintainers-docs for feedback!

Instead of predefining ~/assets and baseUrl in our tsconfig presets user
should now define them in their own in their tsconfig.json, as predefined
aliases were too opinionated.

Fixes #7470
@changeset-bot
Copy link

changeset-bot bot commented Aug 3, 2023

🦋 Changeset detected

Latest commit: 130103d

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot added pkg: integration Related to any renderer integration (scope) pkg: astro Related to the core `astro` package (scope) labels Aug 3, 2023
@Princesseuh
Copy link
Member

Superseded by #8113, sorry for not using your PR, I didn't realize you had everything done already. I added you as a co-author of the commit so you'll have proper credit on your profile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: astro Related to the core `astro` package (scope) pkg: integration Related to any renderer integration (scope)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

tsconfig: compilerOptions.baseUrl should probably be "./" ?
2 participants