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

v3_lazyRouteDiscovery does not respect Vite#base configuration #10124

Open
hanford opened this issue Oct 16, 2024 · 3 comments
Open

v3_lazyRouteDiscovery does not respect Vite#base configuration #10124

hanford opened this issue Oct 16, 2024 · 3 comments

Comments

@hanford
Copy link

hanford commented Oct 16, 2024

Reproduction

I’ve noticed that in the latest Remix release, v2.13.1, v3_lazyRouteDiscovery does not respect Vite’s base configuration.

After looking into the source code, it appears that while lazyRouteDiscovery respects Remix’s basename, however the two configuration options are for different purposes.

Ideally, I’d like the __manifest file to be prefixed with Vite’s base configuration (aligning it with all other JS being served).

It's my understanding that all assets are served under this base (similar to how webpack publicPath) if provided; however, __manifest is not, which is causing an error.


An asset that works (respecting Vite's base)
image

v3_lazyRouteDiscovery not respecting the base, thus 404'ing
image

Reproduction:
https://github.com/hanford/remix-fogOfWar-base

System Info

System:
    OS: macOS 15.0
    CPU: (10) arm64 Apple M1 Pro
    Memory: 110.59 MB / 32.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 20.10.0 - /opt/homebrew/bin/node
    Yarn: 1.22.22 - /opt/homebrew/bin/yarn
    npm: 10.2.3 - /opt/homebrew/bin/npm
    pnpm: 7.32.4 - ~/.npm-global/bin/pnpm
    bun: 1.1.22 - ~/.bun/bin/bun
    Watchman: 2024.08.12.00 - /opt/homebrew/bin/watchman
  Browsers:
    Chrome: 129.0.6668.101
    Safari: 18.0
  npmPackages:
    @remix-run/dev: ^2.13.1 => 2.13.1
    @remix-run/node: ^2.13.1 => 2.13.1
    @remix-run/react: ^2.13.1 => 2.13.1
    @remix-run/serve: ^2.13.1 => 2.13.1
    vite: ^5.1.0 => 5.4.9

Used Package Manager

pnpm

Expected Behavior

__manifest should be servered from base + __manifest

Actual Behavior

__manifest doesn't respect Vite's base, which leads to a 404 breaking the future flag

@brophdawg11
Copy link
Contributor

I'm seeing a 404 on https://github.com/hanford/remix-fogOfWar-base - can you check the permissions on that?

After looking into the source code, it appears that while lazyRouteDiscovery respects Remix’s basename, however the two configuration options are for different purposes.

Can you elaborate on your use case? Knowing what you're expected dev/prod setups are would help us ensure we've got the use cases handled properly.

To my recollection, when we did our initial basename implementation we found that during vite dev, basename had to start with base for vite dev to be able to work at all and I thought we had a warning/error in there when it didn't. It looks like when basename === base and when basename.startsWith(base), these __manifest requests work as expected.

@hanford
Copy link
Author

hanford commented Oct 18, 2024

Made the repo public!

My situation is as follows, I have ~10 separate frontend applications being served on different paths...

example.com/employee/* <-- Application one
example.com/admin/* <-- Application two
example.com/feature/*  <-- Application three
...

image

We have a reverse proxy in front of our applications that looks at the path of an incoming requests, and proxies it to the correct place.

These applications link to one another, share session information, UI components, and from a users perspective, it's just one application.

We aren't using Remix's basename, because we don't want our Links to automatically be prefixed within the application. Instead we're naming all of our routes with what would be the basename. e.g.

admin-application/
  /app/routes/
    /admin-route-1.tsx
    /admin-route-2.tsx
    /admin-route-3.tsx

However, we are using Vite#base, so that our application artifacts (JS/other assets) are prefixed with the applications base path so at the router level we know where to send requests for it's assets.


So, we want to enable lazyRouteDiscovery, but need the __manifest.json request to be prefixed with basePath similar to other application artifacts otherwise at the router level, we don't which application the request should go to.


Hopefully this makes sense, happy to provide more details or try alternative Remix/Vite configurations. Today we're actually a full Next.js shop (with our own patches on top of Next.js) and using a combination of Next.js features that enable this whole setup to work. We're trying to emulate the current setup w/ Remix to hopefully migrate some (or all) applications to Remix.

@jrestall
Copy link
Contributor

We also hit this issue when trying to adopt v3_lazyRouteDiscovery. A lot of our apps were created before basename support was added so are using expressjs and the remix routes config prop to add a custom basename to routes instead.

Looks like this means we'll need to adopt basename before migrating to v7. Quite a big change as the basename is passed manually everywhere to redirects and fetchers etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants