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

imported revive function and islands from script into browser is not taking basepath into account #2326

Closed
Bramart opened this issue Feb 20, 2024 · 3 comments · Fixed by #2334

Comments

@Bramart
Copy link

Bramart commented Feb 20, 2024

Trying to add a basepath /neoplayer/ to a deno fresh app, with another app (node project) at the root.
Basepath is setup in deno fresh config and import in main.ts. Everything works fine except the islands.

In the html file loaded in the browser, basepath is applied to src and href (like said in the docs)

image

At the end there is a script that imports the revive function from main.js and islands from their own js files. Those imports are not prefixed with the basepath.

image

So the request is on the root and the js is not load which leads me to this error in the browser because my root application returns a 404 error page :

image

deno version : 1.36.1
deno fresh version : 1.6.5

@deer
Copy link
Contributor

deer commented Feb 20, 2024

Hi @Bramart, I coded this feature. There is a test for it here:

Deno.test("island tests", async (t) => {

which uses:
export default function Home() {

Can you provide some more details? They should all be redirected like this:
Screen Shot 2024-02-20 at 15 30 33

I'll need more information (ideally a repository which easily reproduces the problem) in order to proceed, because from my side it's working. I guess the test is missing something though.

@Bramart
Copy link
Author

Bramart commented Feb 20, 2024

Hi, here is a fresh repo I just create and still facing the same issue with : https://github.com/Bramart/fresh-project (deno fresh example with basepath config ...)

In local, it works for me too, they are redirected (I have interactivity) :

Capture d’écran du 2024-02-20 17-45-31

But once deployed behind another app, they are not (and no interactivity) :

Capture d’écran du 2024-02-20 17-44-44
Capture d’écran du 2024-02-20 17-44-58

In local, the redirection is made by fresh because fresh manage the whole app, but once deployed to a basepath and next to another app, those requests just hit on the root application like they are supposed to, no ?

@deer
Copy link
Contributor

deer commented Feb 22, 2024

You are very right there. I have a fix and I'm working on a test. Oops 😅

marvinhagemeister pushed a commit that referenced this issue Feb 23, 2024
closes #2326

There was a big misunderstanding on my part -- in particular that a
different server might be listening at the root, so the redirect built
into `context.ts` will never happen. The existing test (as mentioned in
the issue) passes, but only because no one else is listening, and fresh
is able to perform the redirect.

I briefly considered writing a docker container to orchestrate this
properly, but then I didn't want to have docker running in order to run
these tests. (My laptop is old and bad.) So instead the two new tests
very thoroughly assert that scripts included in the html are properly
prefixed with the base path. And yes, they fail if the fix is reverted.

As for the fix itself: I just needed to prepend the asset URL with the
base path.
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 a pull request may close this issue.

2 participants