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

getServerSideProps data urls don't respect assetPrefix #11992

Closed
chrisbrantley opened this issue Apr 18, 2020 · 13 comments · Fixed by #15309
Closed

getServerSideProps data urls don't respect assetPrefix #11992

chrisbrantley opened this issue Apr 18, 2020 · 13 comments · Fixed by #15309
Assignees
Milestone

Comments

@chrisbrantley
Copy link

Bug report

Describe the bug

I have assetPrefix set because we are hosting multiple applications on the same domain and use a proxy to route paths.

assetPrefix = "reports"

The static asset urls are correct. Such as /reports/_next/static/development/pages/_app.js?ts=1587171521639.

However, the data fetching URLs are NOT including the prefix: /_next/data/development/reports/appointments/2020-03-18/2020-04-17.json.

To Reproduce

  • Set assetPrefix to anything
  • Create a page with getServerSideProps
  • use the router (or Link) to go to that page on the client
  • Observe in the network request inspector that the data urls do not include the assetPrefix.

Expected behavior

I would expect the assetPrefix to apply to ALL asset urls that start with _next/

@tmayr
Copy link

tmayr commented Apr 22, 2020

Yep, exactly the same is happening here.

I can confirm that locally works fine, the assetPrefix is present, but on production build it goes away. Even going directly to a getServerSideProps route (not only via router or Link)

Is this the same behavior you're observing @chrisbrantley ?

@chrisbrantley
Copy link
Author

@tmayr Yes, that appears to be the behavior I'm seeing. It appears when the client-side request to the data url fails Next.JS falls-back to reloading the entire page to get the server-side-rendered version. So in this case the app still sort of "works" but it's not a good experience and not efficient.

I'm hoping this was just an oversight in implementing these data urls for client-side fetching of server-side props.

@daveskybet
Copy link

To add to this i'm seeing the same behaviour with data urls not being prefixed using getStaticProps too for an SSG site, so might be a wider issue than just getServerSideProps

@daveskybet
Copy link

Poking about with the code (i'm no expert in the codebase), here seems to be the culprit:

https://github.com/zeit/next.js/blob/canary/packages/next/next-server/lib/router/router.ts#L94

Which seems to add an experimental basePath if exists, rather than the assetPrefix as I would have expected (as gets added to the preload tags for the json files)

A quick addition of the assetPrefix to this line fixes my problem (with an ssg site using getStaticProps), but i'm questioning whether we're misunderstanding the expected behaviour (with the basePath addition)?

@davscro
Copy link
Contributor

davscro commented Jul 7, 2020

@daveskybet have you solved this issue? I am experiencing the same thing

@daveskybet
Copy link

daveskybet commented Jul 7, 2020

@davscro No, I ended up not needing assetPrefix once all the bugs with basePath were solved, basePath sorted all JS paths/Links/Router.push's so assetPrefix wasn't required.

Are you needing assetPrefix without basePath? I suspect the original bug still exists. It looks like there's a discussion on how assetPrefix and data urls should work on the above open PR #13456

@davscro
Copy link
Contributor

davscro commented Jul 10, 2020

Hi @daveskybet,

Yes, you were right. I do not need assetPrefix only basePath is enough and nginx location directive. Now everything works perfectly.

Thanks for the hint. 👍🏼

@Timer
Copy link
Member

Timer commented Jul 30, 2020

Please use the new basePath support instead of assetPrefix in Next.js 9.5.1+ for this use case!

@Timer Timer reopened this Jul 30, 2020
@Timer Timer closed this as completed Jul 30, 2020
@Timer Timer added this to the iteration 6 milestone Jul 30, 2020
@pueyo5
Copy link

pueyo5 commented Aug 11, 2020

Please use the new basePath support instead of assetPrefix in Next.js 9.5.1+ for this use case!

@Timer In our case, this is not possible. The basePath is expecting a subfolder, and we want to use a subdomain.
We are using https://cdn.domain.com and for the proposed solution we have to use https://domain.com/cdn.

Can this issue be reopened or do you want me to create a new one?

@yahoohung
Copy link

We have tested on our website and basePath is not suitable for our settings : /[locale]/[category]/[[...slug]]

@petewarman
Copy link

I need the /_next/data/ urls to be prefixed, but I'm not able to use basePath as that also prefixes my page urls (which is something I don't want). So this fix is actually a major problem for me.

@petewarman
Copy link

Started a discussion here to try and find a fix for the problems created by this change #25681

@balazsorban44
Copy link
Member

This issue has been automatically locked due to no recent activity. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@vercel vercel locked as resolved and limited conversation to collaborators Jan 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants