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

Wrong pathname in Server Islands #11498

Closed
1 task
jamesli2021 opened this issue Jul 18, 2024 · 9 comments
Closed
1 task

Wrong pathname in Server Islands #11498

jamesli2021 opened this issue Jul 18, 2024 · 9 comments
Labels
needs repro Issue needs a reproduction

Comments

@jamesli2021
Copy link

Astro Info

Astro                    v4.12.0
Node                     v20.15.0
System                   macOS (arm64)
Package Manager          npm
Output                   server
Adapter                  @astrojs/node
Integrations             @astrojs/tailwind

If this issue only occurs in one browser, which browser is a problem?

No response

Describe the Bug

During testing, Server Islands uses the /_server-islands/OtherPage pathname instead of / when the server:defer directive is set on the component.

<OtherPage server:defer />

OtherPage.astro

console.log(Astro.url.pathname)

Output

/_server-islands/OtherPage

What's the expected result?

Output should be / and original pathname.

Link to Minimal Reproducible Example

NA

Participation

  • I am willing to submit a pull request for this issue.
@github-actions github-actions bot added the needs triage Issue needs to be triaged label Jul 18, 2024
@florian-lefebvre florian-lefebvre added the needs repro Issue needs a reproduction label Jul 18, 2024
Copy link
Contributor

Hello @jamesli2021. Please provide a minimal reproduction using a GitHub repository or StackBlitz. Issues marked with needs repro will be closed if they have no activity within 3 days.

@github-actions github-actions bot removed the needs triage Issue needs to be triaged label Jul 18, 2024
@ematipico
Copy link
Member

ematipico commented Jul 18, 2024

I'm not really sure it's possible, at all. The URL is built from the Request, which is the endpoint of the component.

Please let's discuss this in the roadmap and make sure this should be the actual behaviour or not

@ematipico ematipico added needs response Issue needs response from OP needs discussion Issue needs to be discussed and removed needs repro Issue needs a reproduction labels Jul 18, 2024
@matthewp
Copy link
Contributor

What's the use-case? The url you're seeing is the URL from the server island request. If you need the URL of the page you can pass it as a prop: <OtherPage server:defer path={Astro.url.pathname} />. You can also likely use the Referer header to get it which the browser sends.

@jamesli2021
Copy link
Author

@matthewp This approach works, but it might be less intuitive for developers, especially theme developers. In this case, we should evaluate how they are impacted by this breaking change.

I get another issue with header in <Nav server:defer /> component and my output is server when testing on Server Islands:

12:42:01 [200] POST /_server-islands/Nav 2ms
12:42:01 [WARN] `Astro.request.headers` is unavailable in "static" output mode, and in prerendered pages within "hybrid" and "server" output modes. If you need access to request headers, make sure that `output` is configured as either `"server"` or `output: "hybrid"` in your config file, and that the page accessing the headers is rendered on-demand.
12:42:01 [WARN] `Astro.request.headers` is unavailable in "static" output mode, and in prerendered pages within "hybrid" and "server" output modes. If you need access to request headers, make sure that `output` is configured as either `"server"` or `output: "hybrid"` in your config file, and that the page accessing the headers is rendered on-demand.
12:42:01 [WARN] `Astro.request.headers` is unavailable in "static" output mode, and in prerendered pages within "hybrid" and "server" output modes. If you need access to request headers, make sure that `output` is configured as either `"server"` or `output: "hybrid"` in your config file, and that the page accessing the headers is rendered on-demand.

@ematipico ematipico added needs repro Issue needs a reproduction and removed needs response Issue needs response from OP needs discussion Issue needs to be discussed labels Jul 19, 2024
Copy link
Contributor

Hello @jamesli2021. Please provide a minimal reproduction using a GitHub repository or StackBlitz. Issues marked with needs repro will be closed if they have no activity within 3 days.

@ematipico
Copy link
Member

ematipico commented Jul 19, 2024

This is a new feature that has different requirements, limitations and use cases that aren't defined yet, so it's a bit early to call it "breaking change", especially when it's opt-in.

Can you provide a reproduction for your issue?

@jamesli2021
Copy link
Author

@matthewp In dev mode, the path will return undefined, and return / in build mode.

session return undefined on both dev and build mode.

<Nav server:defer pathname={Astro.url.pathname} session={Astro.cookies.get('session')?.value />

@ematipico I like to tackle on Astro.object and middleware. If that's one idea, could we have output: 'islands' allow Astro and middlware to be used? It's what made possible for e-commerce, best of both world.

@matthewp
Copy link
Contributor

You should be able to user middleware and get cookies. If you are having an issue there it's likely a bug. Islands should get all of the things that server gets. Please provide an example if you can create one, thank you!

@Mouaz
Copy link

Mouaz commented Sep 17, 2024

As a workaround, you may send headers if you intercept and manipulate window.fetch because it's used to get server island.

// Store the original fetch function
const originalFetch = window.fetch;

// Create a wrapper function with custom headers
window.fetch = async function (url, options) {
  // Create a copy of the original options to avoid modifying them directly
  const fetchOptions = { ...options };

  // Add custom headers if not already present
  if (!fetchOptions.headers) {
    fetchOptions.headers = new Headers();
  }

  // Add your custom header(s)
  fetchOptions.headers.append('Custom-Header', 'Custom Value');
 // in this case we want to add pathname
  fetchOptions.headers.append('x-astro-pathname', window.location.pathname);

  // Call the original fetch function with modified options
  return originalFetch(url, fetchOptions);
};

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs repro Issue needs a reproduction
Projects
None yet
Development

No branches or pull requests

5 participants