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

Support tsconfig.json & jsconfig.json aliases #1747

Merged
merged 15 commits into from Nov 9, 2021
Merged

Support tsconfig.json & jsconfig.json aliases #1747

merged 15 commits into from Nov 9, 2021

Conversation

jonathantneal
Copy link
Contributor

@jonathantneal jonathantneal commented Nov 5, 2021

Changes

This PR adds support for path aliasing from tsconfig.json or jsconfig.json, matching the behavior of the Astro VSCode extension and NextJS.

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

Resolves #1749

This work can supersede #1749, as this provides a stronger developer experience, and creates less confusion between the existing support in *config.json files or thru vite.resolve.alias in astro.config.js.

Testing

This was tested manually.

Docs

Documentation in #1751.

@jonathantneal jonathantneal requested a review from a team as a code owner November 5, 2021 04:19
@changeset-bot
Copy link

changeset-bot bot commented Nov 5, 2021

⚠️ No Changeset found

Latest commit: d67bf6a

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

@jonathantneal jonathantneal marked this pull request as draft November 5, 2021 04:19
@jonathantneal jonathantneal changed the title Improve aliasing Support configuration aliases Nov 5, 2021
@jonathantneal jonathantneal changed the title Support configuration aliases Support tsconfig.json & jsconfig.json aliases Nov 5, 2021
@jonathantneal jonathantneal marked this pull request as ready for review November 5, 2021 14:14
packages/astro/src/core/ssr/resolve-config-paths.ts Outdated Show resolved Hide resolved
packages/astro/src/core/ssr/resolve-config-paths.ts Outdated Show resolved Hide resolved
packages/astro/src/core/ssr/resolve-config-paths.ts Outdated Show resolved Hide resolved
packages/astro/src/core/ssr/resolve-config-paths.ts Outdated Show resolved Hide resolved
packages/astro/src/core/ssr/resolve-config-paths.ts Outdated Show resolved Hide resolved
packages/astro/src/core/create-vite.ts Outdated Show resolved Hide resolved
@natemoo-re
Copy link
Member

@jonathantneal Just took a look at WMR and they have a really nice plugin for this! Much more straightforward. Thoughts on adopting this plugin?

https://github.com/preactjs/wmr/blob/main/packages/wmr/src/plugins/tsconfig-paths-plugin.js

@jonathantneal
Copy link
Contributor Author

@natemoo-re, well, not particularly, but only because WMR moves the resolution complexity to tsconfig-paths, which lacks support for extends or jsconfig.json; both of which tsconfig-resolver and our own VSCode extension support (and so does NextJS).

I ran into this when I was first researching a solution. :(

Copy link
Member

@natemoo-re natemoo-re left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! I had a few small nits about comment clarity, but nothing blocking. Merge when ready.

Comment on lines 62 to 68
aliases.push({
find: /^(?!\.*\/)(.+)$/,
replacement: `${baseUrl
.split('')
.map((segment) => (segment === '$' ? '$$' : segment))
.join('')}/$1`,
});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to look this up because I wasn't sure it was correct, but it is!

I would possibly add some more context to the comment above, and possibly include this link: https://www.typescriptlang.org/tsconfig#baseUrl

The baseUrl option changes the way non-relative specifiers are resolved, so if baseUrl exists, all non-relative specifiers will be resolved relative to the baseUrl.


// compile any alias expressions and push them to the list
for (let [alias, values] of Object.entries(Object(compilerOptions.paths) as { [key: string]: string[] })) {
values = [].concat(values as never);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why this is needed, can you elaborate?

/** Resolved source id from existing resolvers. */
const resolvedId = await this.resolve(source, importer, { skipSelf: true, ...options });

// if the existing resolvers find the file, return that resolution
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This gives priority to other resolvers (like the built-in Node resolution), correct? Might clarify in the comment.

@jonathantneal jonathantneal merged commit ba38147 into withastro:main Nov 9, 2021
@jonathantneal jonathantneal deleted the jn.resolve-from-ts-js-config branch November 9, 2021 17:57
SiriousHunter pushed a commit to SiriousHunter/astro that referenced this pull request Feb 3, 2023
* Resolve paths from tsconfig or jsconfig

https://code.visualstudio.com/docs/languages/jsconfig
https://nextjs.org/docs/advanced-features/module-path-aliases

* edit: rename plugin to `@astrojs/vite-plugin-tsconfig-alias`

Co-authored-by: Nate Moore <natemoo-re@users.noreply.github.com>

* edit: switch from `ps` to `path.posix`

* edit: move sanitization of paths to loop

* edit: rename `resolveConfigPaths` to `configAliasVitePlugin`

* edit: update implementation based on feedback

* prettier

* edit: rename `matchTailingAsterisk` to `matchTrailingAsterisk`

* edit: cleanup with comments

* edit: spellcheck `condition` to `conditionally`

* edit: refactor based on feedback

* edit: Update README.md

* edit: cleanup baseUrl transformation and add explainer comments

* edit: cleanup resolutions and add commenting

* yarn lint

Co-authored-by: Nate Moore <natemoo-re@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants