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

vite fix custom base url #2046

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

patricklx
Copy link
Contributor

@patricklx patricklx commented Jul 14, 2024

this is for vite build.
Vite does only prefix base path to known public files. So, they do have to exist.
Which does not work for virtual vendor.js and others that are generated later in the process
https://github.com/vitejs/vite/blob/fe30349d350ef08bccd56404ccc3e6d6e0a2e156/packages/vite/src/node/plugins/html.ts#L456

also, any custom middleware needs to handle this, since base path can be setup during dev

fixes #2189

@void-mAlex
Copy link
Collaborator

how about making all those URLs relative instead?

@patricklx
Copy link
Contributor Author

how about making all those URLs relative instead?

that doesn't work, because website can open at some level deeper.
e.g my-website.com/my-app/current-page
and then that doesn't work anymore

@patricklx
Copy link
Contributor Author

Maybe that vendor.js could be removed anyway. And directly import loaderjs in entry point?
Other things can also all be imported directly? It would break backwards compatibility though

@void-mAlex
Copy link
Collaborator

my-website.com/my-app/current-page
is exactly why it should be relative
today, the no1 reason why you can't use ./ for root_url is because of the testing framework and how that assumes things to be available
but making everything relative is the way to go

@patricklx
Copy link
Contributor Author

But then it would access ./my-page/current-page/asset/some-asset.js ?
For a src=./asset/some-asset.js

@void-mAlex
Copy link
Collaborator

But then it would access ./my-page/current-page/asset/some-asset.js ? For a src=./asset/some-asset.js

why would it?
it'a all about build time relativity and relative to initial html

@patricklx
Copy link
Contributor Author

patricklx commented Jul 14, 2024 via email

@patricklx
Copy link
Contributor Author

patricklx commented Jul 14, 2024 via email

@void-mAlex
Copy link
Collaborator

Vite keeps it like that in the output..

yes, have it relative to begin with and you don't have an issue ( if there is somewhere I can see this falling short in today's world is the router) but that has problems regardless so you lose nothing by having always relative urls which should be the default as it enables exactly the kind of deploy you mentioned of my-website.com/my-app/

@ef4
Copy link
Contributor

ef4 commented Aug 9, 2024

vendor.js has to remain a script, not a module. That's why it continues to exist as a separate thing. All the code in it expects to run in script context.

@patricklx
Copy link
Contributor Author

i found that this is already done automatically by vite. https://vitejs.dev/guide/build.html#public-base-path
not sure why it was not working for me

@patricklx patricklx closed this Sep 17, 2024
@patricklx patricklx deleted the add-base-url branch September 17, 2024 08:17
@patricklx patricklx restored the add-base-url branch November 1, 2024 18:55
@patricklx
Copy link
Contributor Author

Reopening.
Vite does only prefix base path to known public files.
Which does not work for virtual vendor.js
https://github.com/vitejs/vite/blob/fe30349d350ef08bccd56404ccc3e6d6e0a2e156/packages/vite/src/node/plugins/html.ts#L456

@patricklx patricklx reopened this Nov 1, 2024
@patricklx patricklx force-pushed the add-base-url branch 6 times, most recently from 73e88a8 to fbc0bd3 Compare November 5, 2024 12:44
@patricklx patricklx changed the title vite add BASE_URL vite fix custom base url Nov 5, 2024
@void-mAlex
Copy link
Collaborator

I would hate for us to re introduce this root url concept in vite world
between base html tag and using relative paths we have better tools to deal wiht this without adding back what I consider broken concepts

@patricklx
Copy link
Contributor Author

I would hate for us to re introduce this root url concept in vite world between base html tag and using relative paths we have better tools to deal wiht this without adding back what I consider broken concepts

i had older code still there. have a look now

@patricklx patricklx force-pushed the add-base-url branch 8 times, most recently from 10530f3 to b27682c Compare December 2, 2024 16:55
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.

[Vite] @embroider/virtual/vendor.js is not covered properly by Vite asset handling
3 participants