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

Add get-pathname function #59909

Open
wants to merge 3 commits into
base: canary
Choose a base branch
from
Open

Add get-pathname function #59909

wants to merge 3 commits into from

Conversation

vordgi
Copy link
Contributor

@vordgi vordgi commented Dec 24, 2023

Since this PR is not being merged, I'll leave a recommendation to use @nimpl/getters at the top here.


Adding a feature

What?

Ability to obtain the path in server components

Why?

I understand the concept of RSC - to build server-side and reuse, but restricting the retrieval of data about the current page internally seems excessive to me. Currently, we have to pass props with paths or keys and params through multiple levels of nesting.

For example, these are server-side translations (which require a lang param), pre-assembled card blocks with slight variations in several cards depending on the page, and different banners for individual pages.

Now people use headers, stores and other crutches to solve this - #50405, #43704, stack overflow 1, stack overflow 2

Also, my examples can be solved through a client-side component that returns one of the two passed server components or translations, but we don’t wanna pass all variations to the client. It can be done dynamically, but these data are needed on the page immediately for maximum SEO.

In the result, we are not creating an identical component because we pass unique data through props.

At the same time, I am not very fond of the idea of extending the hook - on the server, it will not be a hook, these are specifically getters. Moreover, a client-side hook can be synchronous, while a server-side getter for the same task can be asynchronous.

How?

I added a new function called getPathname to the next/navigation module.

The logic of the getter replicates the logic of preparing the path in the context of the usePathname hook. I also added an error for server-side getters, stating that they cannot be used on the client.

Fixes #50405, #43704 (in the part about the pathname)

@ijjk
Copy link
Member

ijjk commented Dec 24, 2023

Allow CI Workflow Run

  • approve CI run for commit: 5f41951

Note: this should only be enabled once the PR is ready to go and can only be enabled by a maintainer

1 similar comment
@ijjk
Copy link
Member

ijjk commented Dec 24, 2023

Allow CI Workflow Run

  • approve CI run for commit: 5f41951

Note: this should only be enabled once the PR is ready to go and can only be enabled by a maintainer

@ijjk
Copy link
Member

ijjk commented Dec 24, 2023

Allow CI Workflow Run

  • approve CI run for commit: 634ba2e

Note: this should only be enabled once the PR is ready to go and can only be enabled by a maintainer

@0xbid
Copy link

0xbid commented Dec 26, 2023

Thank you so much for this @vordgi! We desperately need this in our project as the old workarounds no longer work.

@andreibarabas
Copy link

Thanks @vordgi for the initiative for this server feature

@mdahiemstra
Copy link

Thanks for the work @vordgi! I was surprised to learn this was non existent in NextJS.

@rayli09
Copy link

rayli09 commented Jan 11, 2024

Great work, this is unblocking all i18n apps using app router.

@0xbid
Copy link

0xbid commented Jan 21, 2024

@ijjk @timneutkens can we please merge this ASAP? This fixes one of the most critical issues people have with Nextjs, and people are resorting to forking #43704

@vordgi
Copy link
Contributor Author

vordgi commented Jan 21, 2024

For everyone who came to this PR not from discussions - there is no need to fork next.js, I made a package with this function (and a couple of others) and you can use it to check the getter - https://github.com/vordgi/next-impl-getters

P.S. Next.js team, I think for v14.2, it’s probably time to make server getters!

UPD: The new package update includes server contexts

@0xbid
Copy link

0xbid commented Feb 19, 2024

wen merge

@vordgi
Copy link
Contributor Author

vordgi commented May 17, 2024

One more attempt to solve problem - getPageContext PR#65897

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

Successfully merging this pull request may close these issues.

8 participants