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 alias paths on vite #9242

Closed
wants to merge 7 commits into from

Conversation

esteban-url
Copy link
Contributor

Since we've moved to vite as the bundler for the web side, we need to adjust the way we handle the values.

not sure this is the best way to do it, but it does provide support for alias paths and fixes #8666

Signed-off-by: Esteban Rojas <14810250+esteban-url@users.noreply.github.com>
@dac09 dac09 self-requested a review September 30, 2023 20:29
@esteban-url
Copy link
Contributor Author

@dac09 I was thinking that it's maybe a good idea to keep the old implementation for legacy (webpack) projects.

I've added support for both bundlers now. please let me know if there's anything else i might be overlooking.

true
)
expect(ensurePosixPath(webPaths['@ui'])).toMatchInlineSnapshot(
`"./src/ui"`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hey @esteban-url just to check - why is the ./ required for vite? src/ui should still be working I believe?

@dac09
Copy link
Collaborator

dac09 commented Oct 16, 2023

Looking good esteban, but couple of things worth discussing:

  1. Should we be considering moving to https://vitejs.dev/config/shared-options.html#resolve-alias?
  2. What happens for pre-rendered pages here? I think if you are prerendering and using Vite, it may break with this latest change.

Both discussion points, not suggesting we do either just yet

@dac09
Copy link
Collaborator

dac09 commented Nov 23, 2023

Hey @esteban-url picking this up again.

So I've taken another look at this, and I think maybe the module aliases are broken with prerender (even without the change in this PR).

Any chance you'll have time to take a look again? Totally understand if you're busy! Let me know either way ✌️

@dac09
Copy link
Collaborator

dac09 commented Nov 24, 2023

Closing this as superceded by #9574

Thank you again esteban!

@dac09 dac09 closed this Nov 24, 2023
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.

[Bug?]: Vite doesn't support ts path aliases
2 participants