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

Is it possible to have also non-component files under pages/ directory? #26554

Closed
robertjk opened this issue Aug 19, 2020 · 10 comments
Closed

Is it possible to have also non-component files under pages/ directory? #26554

robertjk opened this issue Aug 19, 2020 · 10 comments
Labels
type: question or discussion Issue discussing or asking a question about Gatsby

Comments

@robertjk
Copy link

robertjk commented Aug 19, 2020

Summary

Is it really invalid to have a non-component JavaScript file anywhere under src/pages/?

Relevant information

Today I separated some constants from a page component into a separate file, and put that file in the same directory as page component.

Directory structure

This is the structure of the directory with that page component:

src/pages/
|-- page/
    |-- Page.js
    |-- index.js
    |-- someOtherFile.js

File contents

And these are the contents of the files in there:

// Page.js

import { A, B, (...) } from "./someOtherFile";

function Page() {
  (...)
}

export default Page;
// someOtherFile.js

const A = (...);
const B = (...);
(...)

export { A, B, (...) };
// index.js

import Page from "./Page";
export default Page;

Now build fails

After doing that the build started to fail with the following error message:

A page component must export a React component for it to be valid. Please make sure this file exports a React component:

undefined

As soon as I remove someOtherFile.js the build starts to succeed again. So I assume the problem is that Gatsby treats someOtherFile.js as a page component and tries to build a page from it.

Question

Is that expected behavior and what I'm doing is simply illegal, or is that a bug that should be reported and fixed?

@robertjk robertjk added the type: question or discussion Issue discussing or asking a question about Gatsby label Aug 19, 2020
@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Aug 19, 2020
@AdamSiekierski
Copy link
Contributor

AdamSiekierski commented Aug 19, 2020

Why would you want to do that? The pages directory is meant for only the pages components, if you want to store some utility functions, maybe store them in the src/utils directory?

@robertjk
Copy link
Author

robertjk commented Aug 19, 2020

Why would you want to do that?

Because I find that clean scalable architecture. I keep the files as close as possible to where they are used, instead of putting them in some shared directories, even though they are used only in a single place.

I also do the same for components that are only used locally on some page - are not library components, which are shared between multiple pages. For example:

src/pages/
|-- page/
    |-- PageSectionA/
    |   |-- PageSectionA.js
    |   |-- index.js
    |
    |-- PageSectionB/
    |   |-- PageSectionB.js
    |   |-- index.js
    |
    |-- Page.js
    |-- index.js

maybe store them in the src/utils directory?

It's a possible solution, but I don't like this approach. It keeps things far from it's usage, which makes understanding the relations between files/resources and the maintenance harder.

For example if I want to remove that page completely, in the current architecture, I just need to remove src/pages/page directory and I'm done. If I keep someOtherFile.js in src/utils/, I'll need to remove src/pages/page/ and also src/utils/someOtherFile.js separately. The second is possible to be forgotten/missed, which would result in an unused file src/utils/someOtherFile.js being left in the codebase. For the same reason that architectural pattern makes it easier to track unused files.

Another advantage is that when I'm browsing the code I have all (or at least more) the related files in one directory, instead of needing to jump between multiple different directories (pages/, components/, utils/ etc.).

That being said I still have some stuff in shared directories like src/components/ or src/services/ (or src/utils/), but I only put there stuff that really needs to be shared - stuff that's used in multiple different pages/components. If something is not shared - it's used only in a single place - I keep it locally, in the directory of it's usage.

@simplesessions
Copy link

Not sure if this helps you, but I tend to handle this by having a src/features directory. This allows you to still group functionality that shouldn't be global, but is closely related. This allows a single feature to scale without crowding the global space. And, if something becomes reusable outside of that feature, I promote it to the global src/components dir.

e.g.
src/features/blog/components/Navigation.js
src/features/blog/components/Pagination.js
src/features/blog/helpers/date.js

src/features/sales/components/CheckoutForm.js
src/features/sales/components/InterstitialAd.js
src/features/sales/styles/theme.js

@robertjk
Copy link
Author

robertjk commented Aug 19, 2020

Not sure if this helps you, but I tend to handle this by having a src/features directory. This allows you to still group functionality that shouldn't be global, but is closely related. This allows a single feature to scale without crowding the global space. And, if something becomes reusable outside of that feature, I promote it to the global src/components dir.

e.g.
src/features/blog/components/Navigation.js
src/features/blog/components/Pagination.js
src/features/blog/helpers/date.js

src/features/sales/components/CheckoutForm.js
src/features/sales/components/InterstitialAd.js
src/features/sales/styles/theme.js

Yeah, it's a possible solution and thanks for sharing that. Generally it's the same approach as what I'm doing now, only I keep that structure under pages/ directory and you have a separate features/ directory. I may use exactly the same solution.

But first I want to wait for an answer to my question: Is what I'm doing an illegal thing, or a bug that could be addressed and fixed. If it's the first, I will use the same pattern as you. If it's a bug, then I'll just wait for a fix with some temporary fix for my failing build used in a meanwhile.

@pieh pieh added topic: page creation and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Aug 20, 2020
@pieh
Copy link
Contributor

pieh commented Aug 20, 2020

Gatsby by default adds instance of https://www.gatsbyjs.com/plugins/gatsby-plugin-page-creator/ plugin pointing to <project_directory>/src/pages with some things that ARE ignored (so things starting with _ or . or template-). You could use one of those conventions to mark some files as in fact not a pages (but still, it's kind of similar to moving files out of src/pages - you need to adhere to "hardcoded" convention which doesn't always match the structure of your liking.

But there is a way to add more to the ignore list (thanks to not really documented feature implemented in #17420)

To make use of that feature you would need to add following to your gatsby-config.js (well, you probably already have some plugins, so you would need to merge that with your existing one - the position of page-creator "overwrite" shouldn't matter, so you can append/prepend or put it in the middle):

const path = require(`path`)

module.exports = {
  plugins: [
    {
      // this overwrites default instance of `page-creator` and adds custom `ignore` options
      resolve: `gatsby-plugin-page-creator`,
      options: {
        // the path is important - it needs to point exactly to `<project-dir>/src/pages`,
        // if it doesn't, it will then add another instance of the plugin
        path: path.join(__dirname, `src/pages`),
        ignore: [`someOtherFile.js`]
      }
    }
  ]
}

The ignore option is using https://www.npmjs.com/package/micromatch so you could define some custom "globs" if you would use some convention for naming files that are not meant to be pages.

@robertjk
Copy link
Author

robertjk commented Aug 20, 2020

Gatsby by default adds instance of https://www.gatsbyjs.com/plugins/gatsby-plugin-page-creator/ plugin pointing to <project_directory>/src/pages with some things that ARE ignored (so things starting with _ or . or template-). You could use one of those conventions to mark some files as in fact not a pages (but still, it's kind of similar to moving files out of src/pages - you need to adhere to "hardcoded" convention which doesn't always match the structure of your liking.

But there is a way to add more to the ignore list (thanks to not really documented feature implemented in #17420)

To make use of that feature you would need to add following to your gatsby-config.js (well, you probably already have some plugins, so you would need to merge that with your existing one - the position of page-creator "overwrite" shouldn't matter, so you can append/prepend or put it in the middle):

const path = require(`path`)

module.exports = {
  plugins: [
    {
      // this overwrites default instance of `page-creator` and adds custom `ignore` options
      resolve: `gatsby-plugin-page-creator`,
      options: {
        // the path is important - it needs to point exactly to `<project-dir>/src/pages`,
        // if it doesn't, it will then add another instance of the plugin
        path: path.join(__dirname, `src/pages`),
        ignore: [`someOtherFile.js`]
      }
    }
  ]
}

The ignore option is using https://www.npmjs.com/package/micromatch so you could define some custom "globs" if you would use some convention for naming files that are not meant to be pages.

Thanks. That's a useful information.

That assures me that Gatsby is meant to have only page component files in src/pages/ directory. I'll do what @simplesessions suggested, which is moving src/pages/ to src/features/ and importing those components in files in src/pages/:

src/
|-- pages/
|   |-- page-1.js
|   |-- page-2.js
|   (...)
|
|-- features/
|   |-- Page1/
|   |    |-- Page1.js
|   |    |-- index.js
|   |
|   |-- Page2/
|   |    |-- Page2.js
|   |    |-- index.js
|   |
|   (...)
// src/pages/page-1.js
export { default } from "~features/Page1";
// src/features/Page1/Page1.js

function Page1() {
  (...)
}

export default Page1;
// src/features/Page1/index.js
export { default } from "./Page1";

@pieh
Copy link
Contributor

pieh commented Aug 21, 2020

I just described current state of things and what you could try using existing features - there is always opportunity to add some additional / nicer ways of marking some files as not meant to be pages - but we would have to be careful about this to not cause any breaking changes here.

First thing that comes to mind is to possibly use something like "directive" in the component file content (similar to eslint-ignore / prettier-ignore / ts-ignore) that Gatsby could recognize - so something like // gatsby-page-ignore - we already do check content of the file (this is how we even throw this error that you mentioned in the issue description) -

// Validate that the page component imports React and exports something
// (hopefully a component).
//
if (
!component.includes(`/.cache/`) &&
process.env.NODE_ENV === `production`
) {
const fileContent = fs.readFileSync(component, `utf-8`)
if (fileContent === ``) {
const relativePath = path.relative(directory, component)
return {
error: {
id: `11327`,
context: {
relativePath,
},
},
panicOnBuild: true,
}
}
// this check only applies to js and ts, not mdx
if ([`.js`, `.jsx`, `.ts`, `.tsx`].includes(path.extname(component))) {
const includesDefaultExport =
fileContent.includes(`export default`) ||
fileContent.includes(`module.exports`) ||
fileContent.includes(`exports.default`) ||
fileContent.includes(`exports["default"]`) ||
fileContent.match(/export \{.* as default.*\}/s) ||
fileContent.match(/export \{\s*default\s*\}/s)
if (!includesDefaultExport) {
return {
error: {
id: `11328`,
context: {
component,
},
},
panicOnBuild: true,
}
}
}
}
validationCache.add(component)
return {}
}

So adding additional check for directive like that wouldn't add much of overhead. It does gets kind of tricky, because the error come from gatsby "core" package, but the pages are created actually by a plugin (which is just auto-included in default configuration) so interaction of that directive with pages programatically created in createPages hook could be weird there (someone calls createPage, but it doesn't actually happen because of directive - should this work this way?)

But this is just one idea, there likely are better (or just different) ways to improve DX here.

@robertjk
Copy link
Author

robertjk commented Aug 25, 2020

Having separate directory src/features/ (or any other name you'd prefer) seems like the best solution for my needs. The reason is that for Gatsby it is an unknown directory and so Gatsby doesn't put any restrictions on it's structure. Even if you add gatsby-ignore directive, or something similar, src/pages/ directory will still have some other restrictions or side-effects, e.g. any nested directory will create a new URL route. The only way to "be free" and have things organized by page/feature seems to maintain another directory. And things can still stay very clean and well organized, if I keep the src/pages/ directory flat and doing nothing more than import and export components from src/features/.

@kingluddite
Copy link

I'm trying to follow this pattern but how do I deal with page level GraphQL? If I keep in at the page level src/pages/about.js I get this error "any GraphQL fragments or queries in this file were not processed."

I move it into the features/AboutPage.js and get the same error. I tried to change it to a Static Query and the page did not render. Any ideas or guidance?

export const query = graphql`
  query AboutPageQuery {
    aboutPage: sanityPage(name: { eq: "About" }) {
      name
      headingOne
      expertise
        bioTextBlock: _rawContent
      }
    }
  }
`

@barbalex
Copy link

barbalex commented Nov 17, 2022

I have been adding additional files just as @robertjk wanted to.

Weird things have happened since. One of which: updating things in these files sometimes did not change what dev was presenting. I tried multiple things to get the dev ui to update:

  • restart dev
  • restart the pc
  • gatsby clean
  • nuking all local stuff in the application section of chrome's dev tools

Nothing helped.

Until I found this issue and renamed my files to begin with underscores.

Maybe this can help others encountering the weird problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: question or discussion Issue discussing or asking a question about Gatsby
Projects
None yet
Development

No branches or pull requests

6 participants