Regroup non-static files #6285

brotzky · 2018-07-04T00:31:48Z

Hi,

This is my first attempt at trying to clean up Gatsby's build output. Instead of dumping all the generated files into the root of public by default it puts the scripts and styles into their own directories.

The naming of scripts and styles stems from the variables in the repository.

  public/
      {page}/
      scripts/
      styles/
      static/
      index.html  
      webpack.stats.json

Example output based on "hello-world" gatsby v2

To do

~~Clean up static/ output on each build~~ Not required
Test with images and other static assets
Test with CSS output
Enabled through a feature flag for real world testing

m-allanson

Nice!

Re: clean up static output on each build. We actually don't want to do that. Maybe that will change in the future but I think that's out of scope for this PR. See discussion in #529, @pieh's comment in particular.

This could be a breaking change for plugins that rely on the current public directory structure. I wonder if this should be enabled via a feature flag so it can get some solid real-world testing?

m-allanson · 2018-07-04T10:38:12Z

packages/gatsby/cache-dir/static-entry.js

@@ -221,7 +222,7 @@ export default (pagePath, callback) => {
          as="script"
          rel={script.rel}
          key={script.name}
-          href={urlJoin(pathPrefix, script.name)}
+          href={urlJoin(`scripts`, pathPrefix, script.name)}


I think pathPrefix should be first here. We'll need to ensure this PR works for sites using path prefixes. More info at https://www.gatsbyjs.org/docs/path-prefix/

You can test this locally by setting a pathPrefix in gatsby-config.js and running gatsby build --prefix-paths

Yes, you're correct that the pathPrefix should be leading. I've updated the locations that had it reversed.

I've also tested it locally.

module.exports = { pathPrefix: `/blog` };

Output links become

# For gatsby Links # Without prefix /jobs # With prefix /blog/jobs # For file paths # Without prefix /scripts/component---src-pages-index-js-35049c33efafbda27bb4.js # With prefix /blog/scripts/component---src-pages-index-js-35049c33efafbda27bb4.js

@m-allanson, to confirm, when running gatsby serve locally on a build that has pathPrefix enabled it shouldn't work? All the assets return 404 because Gatsby is serving the the public folder without a prefix (/blog)?

@brotzky yep, that sounds right. Check out the notes here on how to make this work for testing: https://github.com/gatsbyjs/gatsby/tree/master/examples/using-path-prefix

m-allanson · 2018-07-04T10:38:29Z

packages/gatsby/cache-dir/static-entry.js

@@ -252,7 +253,7 @@ export default (pagePath, callback) => {
            as="style"
            rel={style.rel}
            key={style.name}
-            href={urlJoin(pathPrefix, style.name)}
+            href={urlJoin(`styles`, pathPrefix, style.name)}


Same comment about pathPrefix

m-allanson · 2018-07-04T10:39:37Z

packages/gatsby/cache-dir/static-entry.js

@@ -291,7 +292,10 @@ export default (pagePath, callback) => {
  // Filter out prefetched bundles as adding them as a script tag
  // would force high priority fetching.
  const bodyScripts = scripts.filter(s => s.rel !== `prefetch`).map(s => {
-    const scriptPath = `${pathPrefix}${JSON.stringify(s.name).slice(1, -1)}`
+    const scriptPath = `/scripts${pathPrefix}${JSON.stringify(s.name).slice(


m-allanson · 2018-07-04T10:43:00Z

packages/gatsby/src/utils/webpack.config.js

          publicPath: program.prefixPaths
-            ? `${store.getState().config.pathPrefix}/`
-            : `/`,
+            ? `/scripts${store.getState().config.pathPrefix}/`


pathPrefix before scripts

m-allanson · 2018-07-04T10:43:34Z

packages/gatsby/src/utils/webpack.config.js

          publicPath: program.prefixPaths
-            ? `${store.getState().config.pathPrefix}/`
-            : `/`,
+            ? `/scripts${store.getState().config.pathPrefix}/`


pathPrefix before scripts

* first attempt at lifecycle sequence docs closes #6197 * address Kyle's review I hope this is right... * address Shannon's comments address Shannon's comments

v2 should import Link directly from gastby

brotzky · 2018-07-04T14:10:07Z

Thanks so much for checking over everything!
Will go in again today and clean it up with a fresh mind and a better understanding of the codebase!

Will not implement "clean up static output on each build"

…tyles/script paths.

brotzky · 2018-07-04T18:24:39Z

packages/gatsby-cli/src/create-cli.js

+          default: false,
+          describe: `Build site without uglifying JS bundles (for debugging).`,
+        })
+        .option(`group-files`, {


I added a flag to enable grouping of files.
gatsby build --group-files

Why a flag? We try to avoid adding config wherever possible.

@KyleAMathews -- my bad!
What's the best way to add a "feature flag" for this? I tried looking through the code but couldn't grasp it right away. Is there a standard way to handle this?

I was trying to address this comment
#6285 (review)

This could be a breaking change for plugins that rely on the current public directory structure. I wonder if this should be enabled via a feature flag so it can get some solid real-world testing?

The flag would be temporary for people to try it out and then enabled by default after some time. After that the config option would be removed. The purpose is to provide a smoother transition with a testing phase before implementing a potential breaking change.

Hmm, there shouldn't be any plugins which directly operate on outputted css/js so I think this change is pretty safe.

@m-allanson @pieh @jquense can you think of any reason we shouldn't just make this change?

w/o the flag

Without the flags the code would be a lot cleaner. Less config options exposed to users and I can remove ifs, globals, and other logic from this PR. I'm in favour of removing the flag if it only impacts 1-2 plugins.

brotzky · 2018-07-04T18:25:59Z

packages/gatsby/src/utils/webpack.config.js

@@ -86,6 +86,25 @@ module.exports = async (
    return hmrBasePath + hmrSuffix
  }

+  function getOutputPaths() {


There was duplicate code in the webpack output configs, so I moved it into a function that also handles the grouping of files.

gatsbybot · 2018-07-04T18:28:59Z

Deploy preview for using-drupal ready!

Built with commit d8a8377

https://deploy-preview-6285--using-drupal.netlify.com

gatsbybot · 2018-07-04T18:29:40Z

Deploy preview for gatsbygram ready!

Built with commit d8a8377

https://deploy-preview-6285--gatsbygram.netlify.com

brotzky · 2018-07-04T19:08:34Z

To extend this, there is the possibility of adding configuration for the naming of the folders instead of hardcoding scripts and styles. I've left it off this PR, but it theory it would be possible to add.

#5270

brotzky · 2018-07-05T16:08:15Z

Let me know if there's anything else required to get this merged in 👍

Here's the output I'm generating with a local sample project.

@calcsam

@calcsam ready!

szimek · 2018-07-06T06:17:10Z

While it’s great to clean it up, it will make it probably harder to implement a feature like #5270, which would be really useful in cases where there are multiple apps hosted at the same domain.

* First shot at fixing #6295 * basically `section.js` becomes `item.js`—we don't expect every top-level sidebar `yaml` item to be "just" a container for a "section" (or group) of subitems anymore * got rid of the "base directory" setting in `section-list.js` to allow linking to pages not living in the main directory associated with the current sidebar (e.g. `docs` for `doc-links.yaml`) * solving the need of being able to put a link to "Tutorial" in the sidebar of "Docs": https://github.com/gatsbyjs/gatsby/blob/ba55b66e169a16bfb3333658ad804b7ef0d3d197/www/src/data/sidebars/doc-links.yaml#L7-L8 * all links in the sidebar `yaml` files have to be explicit again, e.g. `/docs/` can't be ommitted anymore; that allows the code to be quite a bit simpler * fix false multiple active items on the "Features" page * adjust sidebar item layout a bit * restore `doc-links.yaml` to fit current docs, moved some things for testing * naming is confusing in a couple of places right now * Fix component name * Fix false multiple active items on the "Features" page * move getActiveItem from item.js to body.js and loop over all items in sidebar (as opposed to run getActiveItem on each item individually) * rename `subitems` key to `items` * Styles for miles * reduce letter spacing for sidebar „headlines“/accordion toggle buttons (for both showcase and doc sidebar) * darken sidebar horizontal rules a little * a healthy amount of whitespace fixes for the top-level-item/-section mix * quick bespoke spacing fixes for the tutorial via `ui:tutorial` in `yaml` (fixes #6193) * tutorial-style bullets for subitems everywhere * blockMarginBottom for vertical whitespace * Rename * No hover for section title when accordions are disabled * Rename * Rename * Fix "Features" scrollspy * Restore „Docs“ sidebar

- This line didn't make sense in either English or Spanish. - Added title to links in ES version.

* Pass apiRunner into loader * Define apiRunner for tests * add pageresources into props

Initial tracedSVG support did not include fragments for gatsby-image in 2.0, which uses Fixed and Fluid instead of Resolutions and Sizes. Add support for these new fragments.

`<Link>` components currently break jest tests (#5824), as gatsby-browser-entry.js tries to import pages.json from cache-dir. This fails, because pages.json is created in .cache at build time. This PR adds an empty pages.json to cache-dir.

brotzky · 2018-07-06T14:44:30Z

@szimek would the ability to name the folder outputs solve your issue? For example, allowing the ability to name the output for all JS and CSS to be /blog-assets? In that case it would simplify the rules you'd have to create.

#6285 (comment)

I want to respect the minimal configurability approach of Gatsby with this.

KyleAMathews

I don't see much (any?) value in making these directories configurable. Gatsby doesn't care about catering to idiosyncratic aesthetics for outputted code ;-)

It seems reasonable to move js/css into subdirectories. Can see that it'd simplify the experience for people looking around the outputted code + it'd make it easy to write caching rules for js/css.

KyleAMathews · 2018-07-06T16:25:40Z

packages/gatsby/cache-dir/static-entry.js

@@ -55,6 +55,11 @@ const createElement = React.createElement
 export default (pagePath, callback) => {
  const pathPrefix = `${__PATH_PREFIX__}/`

+  // If enabled, group files in their respective folders otherwise generated
+  // non-static files will be left in the root of the output directory.
+  const pathToGroupStyles = __GROUP_FILES__ ? `styles/` : ``


how about we use js/css instead? Those feel more standard to me + they're shorter (save all the bytes).

@KyleAMathews agreed. The original names were based off variables in the code. +1 for saving all the bytes.

…ser during ssr (#6324)

* cache page components and static query components states * Track query addition/removal and text changes between builds * fix redux-state (de)serialization after changing components reducer to map

* Made gatsby-plugin-netlify-cms work with gatsby v2 * gatsby-plugin-netlify-cms: use mini-css-extract-plugin

- gatsby@2.0.0-beta.18 - gatsby-plugin-netlify-cms@2.0.0-beta.4 - gatsby-source-contentful@2.0.1-beta.9

graphic design blogpost

…e globs (#6316) * refactor: add criticlal scripts and links to static file globs * fix incorrect type given to staticFileGlobs

- gatsby-plugin-offline@2.0.0-beta.3

brotzky · 2018-07-06T17:21:26Z

packages/gatsby/cache-dir/static-entry.js

@@ -57,8 +57,8 @@ export default (pagePath, callback) => {

  // If enabled, group files in their respective folders otherwise generated
  // non-static files will be left in the root of the output directory.
-  const pathToGroupStyles = __GROUP_FILES__ ? `styles/` : ``
-  const pathToGroupScripts = __GROUP_FILES__ ? `scripts/` : ``
+  const pathToGroupStyles = __GROUP_FILES__ ? `css/` : ``


@KyleAMathews renamed to css and js

Here's the output now

edit: just noticed deploy failed. Going to fix that.

…tyles/script paths.

…/gatsby into topics/regroup-non-static-files

szimek · 2018-07-06T18:13:38Z

@KyleAMathews @brotzky Here's an example where being able to name output folders is really useful.

We have many frontend apps hosted on a single www.freeletics.com domain. The blog app (actually it's 6 Gatsby apps, each with different pathPrefix, because of the issue we're talking about here) is at www.freeletics.com/:locale/blog/*, there's another app at www.freeletics.com/:locale/targeting/*, another at www.freeletics.com/:locale/press/* and all other paths are handled by yet another, old Angular app. I don't think it's a rare case to have many apps hosted this way, especially in bigger companies.

We've got nginx server in front of it all that decides, based on the path, which app should handle given request. At first, we built our blog app only once for all 6 languages and mounted it at /. While there was no problem with serving HTML files, there was a problem with serving JS files, because Gatsby outputs all JS files directly to public folder, so when a request came for one of Gatsby JS files, like https://www.freeletics.com/app-123456.js, there was no way for us to write proxy rules for it.

We ended up with 6 separate Gatsby apps, each with pathPrefix like /en/blog, /de/blog etc. It causes some issues - it's harder to set it up in development (we build only one language there, it's also faster this way), gatsby serve doesn't work correctly with pathPrefix, so I had to write our own simple dev server, our Dockerfile is much more complicated, we have to clear cache between each build that makes the whole thing slower to build etc.

If I could tell our Gatsby blog app to output all its JS files e.g. to assets-blog-js, so that the request looks like https://www.freeletics.com/assets-blog-js/app-123456.js, then I no longer have problem with writing a proxy rule for it and I could easily host multiple Gatsby and non-Gatsby apps at the same domain.

KyleAMathews · 2018-07-06T18:33:18Z

@szimek with the perf improvements in v2, wouldn't it be easier to just build that as a single app now?

szimek · 2018-07-06T18:54:34Z

@KyleAMathews But it doesn't solve the problem with figuring out which app should handle given request for non-html files. Let's say I do build all languages as a single app. Gatsby will still output all JS files to its public folder. When someone visits e.g. https://www.freeletics.com/en/blog/posts/freeletics-getting-you-football-fit/ and it triggers a request to e.g. https://www.freeletics.com/app-123456.js file, how can I know which app (Gatsby or not) should handle it?

With 6 separate builds and using pathPrefix, all non-html files are scoped by pathPrefix, so requests come for https://www.freeletics.com/en/blog/app-123456.js, https://www.freeletics.com/de/blog/app-654321.js etc., so it's easy to figure out which app should handle it (we've got a single nginx server that handles all blog apps/builds).

KyleAMathews · 2018-07-06T19:29:23Z

There'd only be one app.js for all the pages? I'm not sure I understand.

szimek · 2018-07-08T15:56:16Z

Hopefully, this clarifies it a bit more:

All our apps are built and deployed independently using Docker and Kubernetes. Even though Gatsby generates unique file names based on the content, I can't put all files from all our apps in a single folder and mount it at /.

We've got nginx proxy in front of all our apps and, based on request path, it proxies the request to a specific app. Thus, I need to be able to write rules for each app based on the path, e.g. proxy all request that match ^/[a-z]{2}/blog/ to the blog app (even though we build it 6 times, each time with different pathPrefix, we treat it as a single app). If Gatsby generates non-html files in root folder or subfolders that are not unique, like https://www.freeletics.com/app-123456.js or https://www.freeletics.com/js/app-123456.js, I can't write such rules and can't figure out which app should handle such request.

brotzky · 2018-07-08T18:59:17Z

@szimek thanks for the detailed explainations!
To solve this issue would you have to create a unique output folder name for each Gatsby project? My gut tells me there's a different way to do this other than creating a custom output folder name for each project, but it's hard to fully grasp your problem over text without seeing the full architecture.

In my opinion, custom name output is out of scope of this PR and could be submitted in a new PR if it's deemed to be valuable feature for the long term of Gatsby. It would be better to discuss the API for such a feature and the implementation in a new issue/PR.

Sidenote

Need confirmation on removing the --group-files flag and making this default
Sorry for all the commits, I rebased upstream master onto this branch 😬

KyleAMathews · 2018-07-08T19:09:06Z

Could you figure out why there's so many changes? It seems your rebasing went wrong some where.

Yes, please remove the flag code. Also agree that your question @szimek isn't very related to this PR so let's move ahead with getting this in.

First try at cleaning up build output

4803d61

brotzky mentioned this pull request Jul 4, 2018

Regroup non-static files into a public's subdirectory #3760

Closed

sudodoki and others added 2 commits July 4, 2018 11:28

Fix typo in type annotations (#6288)

582fb41

Fix broken images (#6282)

50dd934

m-allanson reviewed Jul 4, 2018

View reviewed changes

tsiq-swyx and others added 2 commits July 4, 2018 13:17

add first draft of API Lifecycle docs (#6229)

0986a67

* first attempt at lifecycle sequence docs closes #6197 * address Kyle's review I hope this is right... * address Shannon's comments address Shannon's comments

Update adding-tags-and-categories-to-blog-posts.md (#6294)

7ed8da9

v2 should import Link directly from gastby

Adding flag to build, ordering pathPrefix correctly, and cleaned up s…

e1b8be9

…tyles/script paths.

brotzky commented Jul 4, 2018

View reviewed changes

Removing the deletion of files as referenced by m-allanson and pieh

77371b9

brotzky and others added 2 commits July 4, 2018 16:33

Adding support for styles grouping

59cf18a

Update site-showcase-submissions.md concerning featured sites (#6281)

a9bc9b8

brotzky changed the title ~~WIP: Regroup non-static files~~ Regroup non-static files Jul 5, 2018

final draft ready for review

fdcf23a

@calcsam ready!

fk and others added 6 commits July 6, 2018 11:35

Fix typo (#6296)

912751e

- This line didn't make sense in either English or Spanish. - Added title to links in ES version.

Pass apiRunner into loader (#6272)

0513a6c

* Pass apiRunner into loader * Define apiRunner for tests * add pageresources into props

Fix tracedSVG fragment support in Gatsby 2.0 (#6297)

1041f85

Initial tracedSVG support did not include fragments for gatsby-image in 2.0, which uses Fixed and Fluid instead of Resolutions and Sizes. Add support for these new fragments.

Add empty pages.json. (#5824) (#6304)

50cb0bb

`<Link>` components currently break jest tests (#5824), as gatsby-browser-entry.js tries to import pages.json from cache-dir. This fails, because pages.json is created in .cache at build time. This PR adds an empty pages.json to cache-dir.

switch components reducer to use Map for state (#6307)

37d007a

KyleAMathews reviewed Jul 6, 2018

View reviewed changes

return null object for ssr pageRenderer - fixes importing gatsby-brow…

0add064

…ser during ssr (#6324)

pieh and others added 14 commits July 6, 2018 17:37

Track queries between builds (#6322)

f5d143b

* cache page components and static query components states * Track query addition/removal and text changes between builds * fix redux-state (de)serialization after changing components reducer to map

edits suggested by Phil

eb17042

add blog showcase (#6287)

b8c8040

Adding hyperlinks to images

f9f4cd6

Made gatsby-plugin-netlify-cms work with gatsby v2 (#6292)

06e7360

* Made gatsby-plugin-netlify-cms work with gatsby v2 * gatsby-plugin-netlify-cms: use mini-css-extract-plugin

Added site titles

93c3554

added tags

e02df8f

minor edits

6ec8c70

chore(release): Publish

11206e4

- gatsby@2.0.0-beta.18 - gatsby-plugin-netlify-cms@2.0.0-beta.4 - gatsby-source-contentful@2.0.1-beta.9

other minor edits

150d862

Merge pull request #6317 from gatsbyjs/graphic-design-blogpost

3637083

graphic design blogpost

[gatsby-plugin-offline] Add criticlal scripts and links to static fil…

c2bdb2d

…e globs (#6316) * refactor: add criticlal scripts and links to static file globs * fix incorrect type given to staticFileGlobs

chore(release): Publish

6f789f6

- gatsby-plugin-offline@2.0.0-beta.3

renaming output directories to js and css

366bd97

brotzky commented Jul 6, 2018

View reviewed changes

brotzky added 6 commits July 6, 2018 10:32

First try at cleaning up build output

972fb6b

Adding flag to build, ordering pathPrefix correctly, and cleaned up s…

4042ae1

…tyles/script paths.

Removing the deletion of files as referenced by m-allanson and pieh

695cb68

Adding support for styles grouping

699e5ca

renaming output directories to js and css

eed6616

Merge branch 'topics/regroup-non-static-files' of github.com:narative…

d8a8377

…/gatsby into topics/regroup-non-static-files

brotzky closed this Jul 8, 2018

brotzky mentioned this pull request Jul 8, 2018

Topics/regroup non static files #6346

Merged

Regroup non-static files #6285

Regroup non-static files #6285

Conversation

brotzky commented Jul 4, 2018 • edited Loading

Example output based on "hello-world" gatsby v2

To do

m-allanson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brotzky Jul 4, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brotzky commented Jul 4, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brotzky Jul 4, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brotzky Jul 6, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gatsbybot commented Jul 4, 2018 • edited Loading

gatsbybot commented Jul 4, 2018 • edited Loading

brotzky commented Jul 4, 2018

brotzky commented Jul 5, 2018 • edited Loading

szimek commented Jul 6, 2018 • edited Loading

brotzky commented Jul 6, 2018 • edited Loading

KyleAMathews left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brotzky Jul 6, 2018 • edited Loading

Choose a reason for hiding this comment

Here's the output now

szimek commented Jul 6, 2018 • edited Loading

KyleAMathews commented Jul 6, 2018

szimek commented Jul 6, 2018 • edited Loading

KyleAMathews commented Jul 6, 2018

szimek commented Jul 8, 2018 • edited Loading

brotzky commented Jul 8, 2018

KyleAMathews commented Jul 8, 2018

brotzky commented Jul 4, 2018 •

edited

Loading

brotzky Jul 4, 2018 •

edited

Loading

brotzky Jul 4, 2018 •

edited

Loading

brotzky Jul 6, 2018 •

edited

Loading

gatsbybot commented Jul 4, 2018 •

edited

Loading

gatsbybot commented Jul 4, 2018 •

edited

Loading

brotzky commented Jul 5, 2018 •

edited

Loading

szimek commented Jul 6, 2018 •

edited

Loading

brotzky commented Jul 6, 2018 •

edited

Loading

brotzky Jul 6, 2018 •

edited

Loading

szimek commented Jul 6, 2018 •

edited

Loading

szimek commented Jul 6, 2018 •

edited

Loading

szimek commented Jul 8, 2018 •

edited

Loading