lhapaipai / symfony-vite-dev

Monorepo for symfony-vite development
https://symfony-vite.pentatrion.com
Other
22 stars 20 forks source link

Skip devServer lookup if proxy is defined #27

Closed Blackskyliner closed 5 months ago

Blackskyliner commented 5 months ago

There is no need to resolve the dev server if a proxy is defined, because we would not use that value.

The check in itself is useless if we use a proxy as the dev server lookup may resolve to null even though it is perfectly accessible via proxy.

netlify[bot] commented 5 months ago

Deploy Preview for cosmic-bubblegum-aa631a canceled.

Name Link
Latest commit 57fe215c4809c89cb62a2ca64c36e3339b535370
Latest deploy log https://app.netlify.com/sites/cosmic-bubblegum-aa631a/deploys/662fb9a7c8778e000811b194
andyexeter commented 5 months ago

Hi @Blackskyliner - phpstan seems to be failing because of an undefined variable ($base).

Can you fix this please?

andyexeter commented 5 months ago

Hi @Blackskyliner, this change breaking one of my test applications.

For example, if we're looking up foo.jpg and our host is example.org, and our base is /build/ (the default), then resolveDevServer will correctly resolve and request:

http://example.org/build/foo.jpg

However, with this change in place, if we use proxy_origin: http://proxy.example.org the URL will resolve to:

http://proxy.example.orgfoo.jpg

Which causes a Malformed URL error.

I think the base value needs to be included if it is set.

Blackskyliner commented 5 months ago

Sounds reasonable I reworked it and force pushed a clean commit. We don't need 3 commits for this seemingly simple fix. (Where I failed 3 times to do it correctly though...)

andyexeter commented 5 months ago

Thanks @Blackskyliner - looks good to me other than the coding standards failure

Blackskyliner commented 5 months ago

Ah. Will fix as soon as I am in reach of a computer again.

andyexeter commented 5 months ago

I've fixed the CS issues.

@lhapaipai are you happy for me to merge?

lhapaipai commented 5 months ago

of course !! thanks both of you