-
Notifications
You must be signed in to change notification settings - Fork 27k
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 configuration to specify the name of the pages
directory.
#936
Add configuration to specify the name of the pages
directory.
#936
Conversation
lib/prefetch.js
Outdated
@@ -110,7 +110,7 @@ if (hasServiceWorkerSupport()) { | |||
|
|||
function getPrefetchUrl (href) { | |||
let { pathname } = urlParse(href) | |||
const url = `/_next/${__NEXT_DATA__.buildId}/pages${pathname}` | |||
const url = `/_next/${__NEXT_DATA__.buildId}/${__NEXT_DATA__.pagesDirectory}${pathname}` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I quite don't like this change.
We should not need to any changes to the client code.
For that, keep the pages in the .next
as pages
. There's no need for changing that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additionally, with that we only need to change few places.
So, that's better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think keeping ./.next/pages
regardless would work with SSR if you had a deeper-nested custom pages folder (ala views/pages
, see discussion here)
I wonder when you actually need this 🤔 |
@nkzawa I think a shallow reason would be to simply customize the name to
This patch would allow you to set |
I am starting feeling we should not do this. In the case of Next.js, we allow to customize Next.js with webpack and babel. Having the That's why I don't like to take this. |
I'd argue that this isn't really complicating the source — it's removing dozens of hard-coded strings for |
I think I agree with Arunoda. What are some legitimate use cases for
renaming the directory beyond aesthetics that take away from convention and
recognizability?
On Tue, Jan 31, 2017 at 11:43 AM George Pantazis ***@***.***> wrote:
I'd argue that this isn't really complicating the source — it's removing
dozens of hard-coded strings for pages, and centralizing that value in
one place. As a bonus it allows for the end-users to have more control over
directory structure. From a new-user standpoint this is totally optional —
pages is still the default, so nothing really changes.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#936 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAAy8bHrByt3HpylHl7K4jiR_cc6ZcP-ks5rX47ggaJpZM4LxtU5>
.
|
I mean, I'd argue that "aesthetics" (I'd call it clear folder structure, but potato-patatoh) is important for individual implementations. For example, it's officially sanctioned that Next allows custom servers, where you fine-tune the routing. Let's say I'm using express to derive the incoming routes, and direct some of them to an API and others to Next. Let's also say I want to have a separate Next application for standard web and AMP pages (This would be an expansion of the example I'm working on in #913 ). I'd ideally want a folder structure along the lines of:
Making the pages directory configurable would allow me to do that, or tackle any number of other application structures. |
Couple questions about your example:
|
The convenience of having a single server, I suppose. It would depend on the project. For a more prototype-ish server for a hack week I could definitely see benefit to colocating to simplify the setup.
In my head I was thinking that the programatic usage of |
Here's another example. I'm working on a boilerplate for multi-service apps. If you had a service called 'front' and a service called 'back' it would look somewhat like this:
My goal is to have a consistent development experience across the back- and front-end services (plus any others), and some tools for managing them together (deploying or staging them all at once, for example, or performing integration tests across services). For this project, internal consistency between the different services is more important than external consistency with next.js conventions, so I'd like to be able to keep front-end code (including pages) inside a |
There might be two different questions here. On the one hand it's good to separate into micro services in the cloud (e.g separate front end from API), but on the other I understand wishing to keep all the code base in a monorepo. How can these needs be combined? |
Somewhat related, as I'm applying this to my project I'm also seeing a need to customize the output directory (i.e. the
That way I could separately launch a
I could possibly do this by manually changing |
As far as I see it
Maybe Features add up easily. ¯\_(ツ)_/¯ |
I've come around on this. I think we should allow to make it configurable. |
@gcpantazis could you rebase the PR? |
ecedf64
to
4ac96a8
Compare
Rebased, but looks like I'll need to patch to pass tests; I'll fix, verify with our downstream project, and add some more tests (likely Monday). |
@gcpantazis I wonder if these are related to your changes 🤔 |
4ac96a8
to
8ac4013
Compare
Updated to pass tests as of this comment (missed a few introduced references to |
Basic test added. Let me know if this needs anything else, @timneutkens! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great 👍 marked for after 2.0
What is the status of this? |
I'd really like this too. If we're customizing the |
(Related to vercel#936; this is the more important bit for us at Thumbtack, but doing this enables me to restart work on vercel#936 so I wanted to get it in for review.) Currently the output static assets are stored in `/_next`, but this may not be ideal. At Thumbtack, we need to let the reverse proxies route traffic to several servers, including several Next servers eventually, so it's important that we're able to be very specific about where the static files are stored. This enables us to say: `assetDirectory: '/assets/fe/service-foo' in one app, and... `assetDirectory: '/assets/fe/service-bar' in another, and afterwards point traffic for those folders from the reverse proxies to the appropriate server. I'm sure others may have other use-cases for specifying the output directory, but this is our immediate need. As mentioned above, this is related to the configuration update in PR vercel#936, and merging this in enables us to move forward with restarting that task.
@gcpantazis when you say "ready to go", do you mean this feature "is available now as part of the latest code", or do you mean "scheduled for the next release"? Also, to clarity, I'd like to customize the input source folder (e.g. use |
@tashburn Oh, of course, my misunderstanding. Yeah by "ready to go" I just mean that it's ready for review by the maintainers. :) For what it's worth customizing |
(Related to vercel#936; this is the more important bit for us at Thumbtack, but doing this enables me to restart work on vercel#936 so I wanted to get it in for review.) Currently the output static assets are stored in `/_next`, but this may not be ideal. At Thumbtack, we need to let the reverse proxies route traffic to several servers, including several Next servers eventually, so it's important that we're able to be very specific about where the static files are stored. This enables us to say: `assetDirectory: '/assets/fe/service-foo' in one app, and... `assetDirectory: '/assets/fe/service-bar' in another, and afterwards point traffic for those folders from the reverse proxies to the appropriate server. I'm sure others may have other use-cases for specifying the output directory, but this is our immediate need. As mentioned above, this is related to the configuration update in PR vercel#936, and merging this in enables us to move forward with restarting that task.
(Related to vercel#936; this is the more important bit for us at Thumbtack, but doing this enables me to restart work on vercel#936 so I wanted to get it in for review.) Currently the output static assets are stored in `/_next`, but this may not be ideal. At Thumbtack, we need to let the reverse proxies route traffic to several servers, including several Next servers eventually, so it's important that we're able to be very specific about where the static files are stored. This enables us to say: `assetDirectory: '/assets/fe/service-foo' in one app, and... `assetDirectory: '/assets/fe/service-bar' in another, and afterwards point traffic for those folders from the reverse proxies to the appropriate server. I'm sure others may have other use-cases for specifying the output directory, but this is our immediate need. As mentioned above, this is related to the configuration update in PR vercel#936, and merging this in enables us to move forward with restarting that task.
This seems especially important now that we have dynamic imports. I now only have a single This means that the name of the directory is not meaningful for apps that take advantage of dynamic imports, which seems to be an important part of the direction that Next.js is moving. |
Is this in yet ? |
@timneutkens Maybe we should just close this one out and make an issue to track the feature request. As mentioned above this code is too far removed from the active dev branches to be salvaged. #2186 adds some configuration for the |
Hey @gcpantazis. Thank you for all your work on this! Curious if this is still slated for a release -- seems it got caught up in the CI and now there's a couple of conflicts. |
Lets close this, feel free to create an issue for the feature request. |
@gcpantazis @timneutkens Has there been any further investigation or follow-up ticket? |
Any updates on this? |
I no longer (personally) view this as critical or necessary. Much of what the team has done in the past year has made much of what I was proposing here obsolete (i.e. #2186 is largely irrelevant given newer features like https://github.com/zeit/next.js#dynamic-assetprefix)), and most of my rationale above for specifying a custom If anyone disagrees, they're welcome to open a ticket for the feature request! But please do so instead of pinging on this thread. 🙇 👍 👋 |
So... What can we do? |
I suppose you shoud add this. it's such a limitation for some who have their own special folder structure.. for example I like to keep my source code away from package.json, babelrc, ... .
|
In your case this should work:
|
@lavir that does not work for that case. It means nextjs will add .next directory inside src along with other things such as tsconfig (for us ts users) among other things. Those files belong in the root, not in src. We do not want to change the next root, just the pages directory. |
Just leaving this here for reference: https://nextjs.org/blog/next-9-1#src-directory-support |
Thanks @timneutkens I would have missed this and probably only found out way later! |
@timneutkens #30172 If I have many pages below pages. If there is no configured routing table, it cannot be classified, which is difficult to maintain |
I treat 'pages' dir as router. Since this is kind of handicapped router :P. So in page components under pages dir i keep simple PageWrapper copmonents witch provide layout and contain target page component from /components/@pages. Then pages components don't have any logic. Just point to correct component in components/@pages dir. In getStatuc/ServerSideProps i just run prepared data fetch modules witch provides data expected by target page component. sadly i can't rename pages to 'router' or 'routes' |
Being able to customize the name of the
pages
directory was brought up in #914, and this PR makes that possible by adding configuration innext.config.js
.It does bring up the possible need of a refactor for how configuration is disseminated to the server code, as it's becoming very repetitive to have to call
getConfig(dir)
every place we need access to the settings (and this commit seems to be propagating that convention more than ever).So, open for consideration! Happy to continue to work on this, but as @nkzawa mentioned it needs discussion.
cc/ @arunoda @rauchg @timneutkens from the relevant conversation in #914 🤝