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

Ability to co-locate non-page files in the pages directory #8454

Closed
quantizor opened this issue Aug 20, 2019 · 19 comments
Closed

Ability to co-locate non-page files in the pages directory #8454

quantizor opened this issue Aug 20, 2019 · 19 comments

Comments

@quantizor
Copy link
Contributor

quantizor commented Aug 20, 2019

Feature request

Is your feature request related to a problem? Please describe.

There are many different methods of codebase organization in the wild and one popular one is to use an index.js file for the page component itself and surround it with sibling files for various subviews that are specific to that page.

However, next doesn't support this kind of structure because it automatically assumes all files inside of pages/ are real pages.

Describe the solution you'd like

There already is precedent in the codebase for convention over configuration when it comes to filenaming:

  • the [id].js pattern for dynamic routing
  • the _(app|document|error).js pattern for overriding built-in pages

I propose extending the leading-underscore naming pattern such that any file inside pages/ prefixed with an underscore will not be considered as a public page and can simply live in the folder.

pages/
  index.js                  <- page
  accounts/
    index.js                <- page
    _constants.js           <- not a page
    _modal.js               <- not a page

Describe alternatives you've considered

Another prefix I thought of was "$", e.g. $constants.js but this felt a little strange to write and actually is not supported in some filesystems.

@timneutkens
Copy link
Member

However, next doesn't support this kind of structure because it automatically assumes all files inside of pages/ are real pages.

The convention is that all files inside pages are pages, it's quite a simple convention that we're not planning to change as of right now.

Introducing something like _ to mark private pages also has security implications, as there might be accidental exposure of code etc.

Something like this behavior is already possible to achieve by having a separate directory holding this structure and then the public routes would be exposed in pages.

@baer
Copy link

baer commented Aug 26, 2019

My team has struggled with this too mostly because our project started out with all components under pages (like what this issue proposes allowing) and, without a recommended path that I've seen, the migration has taken us down a few unproductive roads.

At present we have the following three ways to set up pages in our codebase. I know that the Next.js team works hard to study how people use the framework and, have a large Next.js codebase internally. If there are patters that you've found work well, I'd love to hear your thoughts.


Write the component in the pages directory
This works well for simple pages like the error page but fails when you need to break a component into sub-components. First is that it's not always the right moment for a refactor to split the page up. Occasionally we've ended up with logic in two places: the pages dir and the supporting files dir which is something like ./src/components/pagename. Second is that, now that we have path-based dynamic URLs, the structure of the pages dir does not match the structure of the dir with supporting files.

./pages/customer/[customerId]/index.js

class Customer extends React.Component {
  state = {
    isLoading: true
  };

  // Get logged-in-only data
  componentDidMount() {
    this.setState({ isLoading: true });
    this.props.getCustomer(this.props.router.customerId)
      .then(() => this.setState({ isLoading: false }));
  }

  render() {
    return (
      <main>
        <div>{this.props.customer.name}</div>
      </main>
    );
  }
}

const mapStateToProps = state => ({
  customer: state.customer || {}
});

const mapDispatchToProps = dispatch => ({
  getCustomer: bindActionCreators(customerStore.actions.fetch, dispatch)
});

export default flow(
  connect(mapStateToProps, mapDispatchToProps),
  withLayout(),
  withRouter()
)(Customer);

Separate the presentation component from data-fetching, pulling IDs out of routes, and wrapping in layout
This works well when complex data tasks like fetching and manipulating the store are limited. For pages with complex needs, separating all of the data manipulations ended up splitting the logic across the wrong b, and we still had to go hunt for what code lived in what file.

./src/components/customer/index.js

const Customer = customer => (
  <main>
    <div>{customer.name}</div>
  </main>
);

export default Customer;

./pages/customer/[customerId]/index.js

class Customer extends React.Component {
  state = {
    isLoading: true
  };

  // Get logged-in-only data
  componentDidMount() {
    this.setState({ isLoading: true });
    this.props.getCustomer(this.props.router.query.customerId)
      .then(() => this.setState({ isLoading: false }));
  }

  render() {
    return <Customer customer={this.props.customer} />;
  }
}

const mapStateToProps = state => ({
  customer: state.customer || {}
});

const mapDispatchToProps = dispatch => ({
  getCustomer: bindActionCreators(customerStore.actions.fetch, dispatch)
});

export default flow(
  connect(mapStateToProps, mapDispatchToProps),
  withLayout()
)(Customer);

Only use components in the pages dir for pulling IDs out of routes and wrapping in layout
This has been where our team ended up. The drawbacks so far are that there is a bit of unnecessary ceremony for small components and that we have to come up with a naming convention for every page since the actual logic lives in /src/components/<component-name>/index.js and those are very manageable problems.

./src/components/customer/index.js

class Customer extends React.Component {
  state = {
    isLoading: true
  };

  // Get logged-in-only data
  componentDidMount() {
    this.setState({ isLoading: true });
    this.props.getCustomer(this.props.id)
      .then(() => this.setState({ isLoading: false }));
  }

  render() {
    return (
      <div>{this.props.customer.name}</div>
    );
  }
}

const mapStateToProps = state => ({
  customer: state.customer || {}
});

const mapDispatchToProps = dispatch => ({
  getCustomer: bindActionCreators(customerStore.actions.fetch, dispatch)
});

export default flow(
  connect(mapStateToProps, mapDispatchToProps),
)(Customer);

./pages/customer/[customerId]/index.js

class Customer extends React.Component {
  render() {
    return (
      <main>
        <Customer id={this.props.router.query.customerId} />
      </main>
    );
  }
}

export default flow(
  withLayout(),
  withRouter()
)(Customer);

@timneutkens
Copy link
Member

timneutkens commented Aug 28, 2019

It's extensively discussed in #3728 so I'm thinking we should probably close this issue.

@baer for zeit.co we use a combination of the first and second method, we have a pages, components and lib directory. Pages generally build up out of components, header, footer etc. and implement the data fetching logic through getInitialProps. There's a bit of duplication but that's not a bad thing as it improves bundling.

@baer
Copy link

baer commented Sep 9, 2019

🤔I'm a bit surprised to hear you're not on the #3 train. Do you have any conventions around keeping components and lib in sync with what's in pages? With path-based routing, you can't match up names anymore.

@timneutkens
Copy link
Member

We utilize the design system everywhere so we try to avoid introducing page specific components. However if there are page specific components they match the pages folder structure.

@baer
Copy link

baer commented Sep 11, 2019

I also use a design system but I'm using Next.js for a dynamic logged-in-only web Application. With the level of complexity for each page, it's impractical to avoid page-specific components. To give you a better sense of what I'm talking about, here's a screenshot of our folder structure.

/next-app/pages

Screen Shot 2019-09-10 at 7 29 34 PM

next-app/src

Screen Shot 2019-09-10 at 7 28 36 PM

You can see that I've got three types of files: page entry points in the /pages directory, common components in the /src/components directory and page-specific components in /src/pages. You can see that we've done our best to name each page to match the route name. This keeps things mostly organized but is always a small annoyance trying to figure out where code lives.

Is it uncommon to build mostly-dynamic apps like this using Next.js? If no, are there other patterns you've seen out there for keeping the project well structured?

Note: Now that project structure drives routing, in addition to compiled pages, and with RFCs like the #8451 and #8063 coming up, directory structure may be an interesting thing to add to #8442.

@timneutkens
Copy link
Member

You can see that I've got three types of files: page entry points in the /pages directory, common components in the /src/components directory and page-specific components in /src/pages. You can see that we've done our best to name each page to match the route name. This keeps things mostly organized but is always a small annoyance trying to figure out where code lives.

So zeit.co is mostly the same except we only have the components directory, so no src/pages.

Is it uncommon to build mostly-dynamic apps like this using Next.js? If no, are there other patterns you've seen out there for keeping the project well structured?

It's not uncommon, however I do think that there's currently no "recommended" folder structure in the docs and we might want to provide some guidance there.

I'll ask some people running larger apps to reply to this issue (if they want to).

Note: Now that project structure drives routing, in addition to compiled pages, and with RFCs like the #8451 and #8063 coming up, directory structure may be an interesting thing to add to #8442.

Great point, we didn't want to send the tree in particular as it could be sensitive info. But checking based on heuristics would be fine I think 🙏

@iaincollins
Copy link
Contributor

This is interesting. Our app at The Economist is also structured the way other projects above are:

root
└── src/
  ├── components/
  ├── lib/
  └── pages/

We actually use the underscore (_) prefix for 'private' (internal, but not actually secret) pages, based on common convention of underscores often being used to denote that.

Personally I don't hate the idea of having underscore denote files are 'private' but there is probably is value in encouraging people not to fill up pages with other files (that are not pages to route to), including from a security PoV (on the basis that people do tend to abuse features in practice).

@shynome
Copy link

shynome commented Dec 2, 2019

@probablyup
this is so easy, set pageExtensions as below

// next.config.js
module.exports = {
  pageExtensions: ["page.tsx", "api.ts"],
}

a file tree like below, and only /list page and /api/pm2 api work

pages/
├── api
│   └── pm2
│       ├── index.api.ts
│       └── pm2.ts
├── _app.page.tsx
├── _document.page.tsx
├── list.page.tsx
├── pm2.client.ts
└── theme.ts

@alihesari
Copy link

alihesari commented Apr 18, 2020

To solve this issue, I added getInitialProps in my component and pass a prop to the component. When the component imported in a page getInitialProps function will not run and that the prop will be undefined so I return the current component and otherwise I return 404 error.

Something like:

/pages
....  /home.js
........  /components
.........  myComp.js

home.js

import MyComp from "./components/myComp";

const home = ({ course }) => {
    return (
        <>
             <h1>Home page</h1>
             <MyComp />
        </>
    );
}

export default home;

mycomp.js

import Error from 'next/error'

const mycomp = ({ render404 }) => {
    return render404 ? <Error statusCode={404} /> :  <h2>This is My component</h2>;
}

mycomp.getInitialProps = (ctx) => {
    return { render404: true };
};

export default mycomp;

@GrantGryczan
Copy link

GrantGryczan commented Apr 7, 2021

this is so easy, set pageExtensions as below

@shynome Good idea, but then, instead of import ... from 'pages/whatever', I have to do import ... from 'pages/whatever/index.page' which is way more verbose and difficult to read. It is not a good long-term solution.

@shynome
Copy link

shynome commented Apr 8, 2021

this is so easy, set pageExtensions as below

@shynome Good idea, but then, instead of import ... from 'pages/whatever', I have to do import ... from 'pages/whatever/index.page' which is way more verbose and difficult to read. It is not a good long-term solution.

@GrantGryczan you still can use import ... from 'pages/whatever', just add pages/whatever/index.ts file for export , index.page.tsx now is just for render page

@GrantGryczan
Copy link

GrantGryczan commented Apr 11, 2021

this is so easy, set pageExtensions as below

@shynome Good idea, but then, instead of import ... from 'pages/whatever', I have to do import ... from 'pages/whatever/index.page' which is way more verbose and difficult to read. It is not a good long-term solution.

@GrantGryczan you still can use import ... from 'pages/whatever', just add pages/whatever/index.ts file for export , index.page.tsx now is just for render page

@shynome Adding an additional (edit: second) file for each page you want to import something from elsewhere isn't any more clean than import ... from 'pages/whatever/index.page'.

It would be better if something for this were built into Next. My point is that I'm not sure why this issue was closed. Your solution certainly was not sufficient to warrant it.

@shynome
Copy link

shynome commented Apr 11, 2021

@GrantGryczan I think one component one file is a good idea, not all components in one file.
but anyway, just follow your favorite way.

@GrantGryczan
Copy link

GrantGryczan commented Apr 11, 2021

@GrantGryczan I think one component one file is a good idea, not all components in one file.
but anyway, just follow your favorite way.

I see how you misunderstood me and have edited my previous comment. I did not suggest putting all components in one file. That would not even be possible with Next considering each page must be a separate file. I suggested one file per page.

What you are suggesting, on the contrary, is one file per page, plus an additional file (two total files) for any page I want to import something from elsewhere. That solution is what I'm saying is no cleaner than import ... from 'pages/whatever/index.page', neither of which I think should warrant this issue being closed.

berndartmueller added a commit to berndartmueller/next-translate that referenced this issue Apr 22, 2021
To be able to use custom Next.js page extensions (vercel/next.js#8454 (comment)), a new property `extensionsRgx` is added to the i18n configuration.

# How to use

// i18n.js

```
module.exports = {
  extensionsRgx: /\.(page|api)\.(tsx|ts|js|mjs|jsx)$/,
  ...
}
```
aralroca pushed a commit to aralroca/next-translate that referenced this issue Apr 22, 2021
* Add new property `extensionsRgx` to config

To be able to use custom Next.js page extensions (vercel/next.js#8454 (comment)), a new property `extensionsRgx` is added to the i18n configuration.

# How to use

// i18n.js

```
module.exports = {
  extensionsRgx: /\.(page|api)\.(tsx|ts|js|mjs|jsx)$/,
  ...
}
```

* docs: add `extensionsRgx` configuration property
@RareSecond
Copy link
Contributor

@GrantGryczan Why would you ever want to import another page elsewhere? Pages are meant to be standalone components.

aralroca added a commit to aralroca/next-translate that referenced this issue May 21, 2021
* replace quotes only for the special $' pattern specific to .replace() (#529)

* replace quotes only for the special $' pattern specific to .replace() instead of replacing quoutes everywhere

* use .replace() callback to avoid parsing special string patterns

* write tests to verify that templateWithHoc and templateWithLoader correctly replaces special string cases. Update snapshots

* Update package version

* fix(transCore): when no suffix don't match spaces (#534)

* Update package version

* _one works (#541)

* Update package version

* Update README.md (#552)

* Update package version

* Update dependencies (#554)

* Update dependencies

* Update example deps

* Update Trans text after change lang (#566)

* Ignore api.(ts|js...) file (#567)

* Add useMemo to useTranslation (#574)

* Update deps (#582)

* Update version of package.json

* Adding tests (#585)

* Add new property `extensionsRgx` to config (#589)

* Add new property `extensionsRgx` to config

To be able to use custom Next.js page extensions (vercel/next.js#8454 (comment)), a new property `extensionsRgx` is added to the i18n configuration.

# How to use

// i18n.js

```
module.exports = {
  extensionsRgx: /\.(page|api)\.(tsx|ts|js|mjs|jsx)$/,
  ...
}
```

* docs: add `extensionsRgx` configuration property

* Update package.json

* Listen for `namespaces` changes and load necessary namespaces (#592)

* Update package.json version

* Revert "Add useMemo to useTranslation" (#605)

This reverts commit 8abc458.

# Conflicts:
#	package.json

* Remove console.warn because is already solved on Next.js 10.2.1-canary.4 (#609)

Already fixed in [Next.js canary 10.2.1-canary.4](https://github.com/vercel/next.js/releases/tag/v10.2.1-canary.4)

Co-authored-by: AndrewB <[email protected]>
Co-authored-by: slevy85 <[email protected]>
Co-authored-by: Justin <[email protected]>
Co-authored-by: Bernd Artmüller <[email protected]>
Co-authored-by: Rihards Ščeredins <[email protected]>
@GrantGryczan
Copy link

GrantGryczan commented May 26, 2021

@GrantGryczan Why would you ever want to import another page elsewhere? Pages are meant to be standalone components.

There are plenty of use cases: importing a custom error page for use on other pages (which is encouraged in the Next.js docs), importing a TypeScript type from a page (e.g. the type for the page's props), importing something else exported from a page (e.g. a React context, a constant value, etc.) which is impractical to put in a separate module and is relevant specifically only to the page exporting it, and so on.

kodiakhq bot pushed a commit that referenced this issue Aug 27, 2021
…omponents (#22740)

Feel free to edit this/suggest changes as you see fit to match the style of your docs or how you want this feature represented.

There seemed to be many issues regarding colocating non-page files with page components in  the `pages` directory where `pageExtensions` was finally suggested as a solution. I think it would be great to document it better since it seems to be a often requested workflow to colocate test, generated files, styles, and other files needed by page components in the `pages` directory. The note that exists in the docs currently alludes to the ability to do this but it doesn't contain much context that would help make it more discoverable.

Relevant issues:
#11113 (reply in thread)
#8454 (comment)
#8617 (comment)
#3728 (comment)
@rperozin
Copy link

@probablyup this is so easy, set pageExtensions as below

// next.config.js
module.exports = {
  pageExtensions: ["page.tsx", "api.ts"],
}

a file tree like below, and only /list page and /api/pm2 api work

pages/
├── api
│   └── pm2
│       ├── index.api.ts
│       └── pm2.ts
├── _app.page.tsx
├── _document.page.tsx
├── list.page.tsx
├── pm2.client.ts
└── theme.ts

Absolutely! The NextJS suggest exactly this on their documentation.
https://nextjs.org/docs/api-reference/next.config.js/custom-page-extensions#including-non-page-files-in-the-pages-directory

@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 27, 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

No branches or pull requests

10 participants