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

Assets path are conflicting with routing #9379

Open
ric980 opened this issue May 6, 2024 · 5 comments
Open

Assets path are conflicting with routing #9379

ric980 opened this issue May 6, 2024 · 5 comments

Comments

@ric980
Copy link

ric980 commented May 6, 2024

Reproduction

It hard to reproduce as it happens infrequently, maybe several times a day, but I'll explain the issue in very detail. I cannot reproduce it locally. But it is reported by Sentry a few times a day.

This is my route: app/routes/($lang).products.$id.
And this is my image path: /images/products/abc1-s@1x.webp. The images for the products are located in public/images/products/....

And this is the error from Sentry:

URL = http://shop:3000/images/products/abc1-s@1x.webp (where abc1 is the ID of the product)
Error = GET routes/($lang).products.$id

The actual error from my ($lang).products.$id.tsx component is Error Invariant failed., because it cannot find a product with that ID.

Why the router matches a path for my assets in the public directory? I understand that this regex $param.products.$param matches this URL /images/products/abc, but it shouldn't. I am asking for an image here.

Please, help! This happens 300 times by now and users cannot see the images. But when they refresh the page - it works.

System Info

System:
    OS: Alpine Linux 3.19.1
  Binaries:
    Node: 20.12.1
    Yarn: 4.0.2
    npm: 10.2.4
  npmPackages:
    @remix-run/dev: ^2.9.1 => 2.9.1
    @remix-run/node: ^2.9.1 => 2.9.1
    @remix-run/react: ^2.9.1 => 2.9.1
    @remix-run/serve: ^2.9.1 => 2.9.1

Used Package Manager

yarn

Expected Behavior

I expect that assets paths won't conflict with routing.

Actual Behavior

Assets paths conflicting with routing.

@brophdawg11
Copy link
Contributor

brophdawg11 commented May 6, 2024

Are you using remix-serve? If so then I think this is due to the default express.static behavior which sets fallthrough:true b default, so if a path doesn't match inside public/ it continues and tries to match that path via subsequent handlers which in this case would be the Remix handler.

remix-serve source code link for reference: https://github.com/remix-run/remix/blob/main/packages/remix-serve/cli.ts#L135

One quick and easy solution is to check params.lang against an accepted list of language codes and return a 404 to short circuit your loader.

If you need more control, you can implement your own express server and set up an express.static handler for /images that has fallthrough:false so it never hits the Remix handler.

@ric980
Copy link
Author

ric980 commented May 9, 2024

Thanks for your help. How to do this:

One quick and easy solution is to check params.lang against an accepted list of language codes and return a 404 to short circuit your loader.

@kiliman
Copy link
Collaborator

kiliman commented May 9, 2024

I think what he means is that in your loader for the ($lang).products.$id.tsx route

export async function loader({ request, params }: LoaderFunctionArgs) {
  validateLangParam(params.lang)
  // ... continue
}

function validateLangParam(lang: string) {
  if (lang === 'images') {
    throw new Response('Not Found', { status: 404 })
  }
  // or you can do 
  if (lang && !(['en', 'fr', 'es', 'de'].includes(lang))) {
    throw new Response('Not Found', { status: 404 })
  }  
}

@brophdawg11 brophdawg11 added the needs-response We need a response from the original author about this issue/PR label May 10, 2024
@ric980
Copy link
Author

ric980 commented May 12, 2024

Thanks, but I definitely don't want to do this in every loader.

Also, hit a new problem. I have these routes:

($lang).contact.tsx
($lang).thanks.$id.tsx

As the lang is optional, now this path /thanks/contact matches this route ($lang).contact.tsx. You should be more greedy when doing regexes. Exact matches should go first. Now I have to rework my URLs to get around all these routing issues.

Is there any way to do something like this:

(en|de).contact.tsx
(en|de).thanks.$id.tsx

@github-actions github-actions bot removed the needs-response We need a response from the original author about this issue/PR label May 12, 2024
@miladhatami1393
Copy link

Good job

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

4 participants