SSR: Fix file-loader assets #36204

sirreal · 2019-09-20T07:32:48Z

Output file-loader files once and share the output files across builds.

Currently, SSR does not show any icons because of a path mismatch.

I suspect this was a regression from #30768 which started to affect icons after #32763

Reported by @michaeldcain

Screens

Before

SSR

Paths like images/file.ext#id

Rendered

Paths like /calypso/evergreen/images/file.ext#id

After

SSR

Paths like /calypso/images/file.ext#id

Rendered

Paths like /calypso/images/file.ext#id

Testing instructions

Visit https://calypso.live/themes?branch=update/fix-ssr-files logged out with JavaScript disabled. You should see icons.
~~Repeat with a legacy browser (ensure JavaScript assets are coming from the fallback URL).~~ Some legacy browsers are rendered without external SVG support so they rely on JavaScript to render icons. Just trust that they work 😅
The same behavior in production results in missing icons.

Output files once to images and share the output across builds.

matticbot · 2019-09-20T07:32:49Z

Test live: https://calypso.live/?branch=update/fix-ssr-files

webpack.config.node.js

matticbot · 2019-09-23T15:24:24Z

Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:

Webpack Runtime (~124 bytes removed 📉 [gzipped])

name      parsed_size           gzip_size
manifest       -471 B  (-0.3%)     -124 B  (-0.4%)

Webpack runtime for loading modules. It is included in the HTML page as an inline script. Is downloaded and parsed every time the app is loaded.

App Entrypoints (~2 bytes removed 📉 [gzipped])

name        parsed_size           gzip_size
entry-main        -44 B  (-0.0%)       -2 B  (-0.0%)

Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used.

Sections (~1606 bytes added 📈 [gzipped])

name            parsed_size           gzip_size
stats                +780 B  (+0.1%)      +29 B  (+0.0%)
woocommerce          +777 B  (+0.0%)      +23 B  (+0.0%)
purchases            +774 B  (+0.1%)      +21 B  (+0.0%)
checkout             +736 B  (+0.1%)     +788 B  (+0.4%)
domains              +704 B  (+0.1%)     +751 B  (+0.3%)
plans                 +54 B  (+0.0%)       -6 B  (-0.0%)
feature-upsell        +39 B  (+0.0%)       +0 B

Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to.

Async-loaded Components (~91 bytes added 📈 [gzipped])

name                          parsed_size           gzip_size
async-load-design-blocks           +777 B  (+0.0%)      +22 B  (+0.0%)
async-load-design-playground       +774 B  (+0.0%)      +25 B  (+0.0%)
async-load-design                  +774 B  (+0.0%)      +44 B  (+0.0%)

React components that are loaded lazily, when a certain part of UI is displayed for the first time.

Legend

What is parsed and gzip size?

Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory.
Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.

Generated by performance advisor bot at iscalypsofastyet.com.

p-jackson

Looks good, works for me.

Tested the IE11 behaviour and got what was expected: gridicons only show in IE11 when JS is enabled.

I thought it'd be better to use a shared constant instead of the '/calypso/images' literal. But from searching around I see that this base url is pretty entrenched in magic strings at this stage 🤷‍♂

xknown · 2019-09-25T14:23:29Z

@sirreal I just rebased the branch I was working on #36069 and I am now getting 404 on a bunch of gridicons related stuff:

HEAD /version?1569421328925 200 0.309 ms - 20
GET /calypso/images/gridicons-84d04a83ed8c3cfc40de995e9bd32649.svg 404 3.268 ms - 4167
GET /calypso/images/gridicons-84d04a83ed8c3cfc40de995e9bd32649.svg 404 4.652 ms - 4167
GET /calypso/images/gridicons-84d04a83ed8c3cfc40de995e9bd32649.svg 404 3.664 ms - 4167
GET /calypso/images/gridicons-84d04a83ed8c3cfc40de995e9bd32649.svg 404 5.827 ms - 4167
HEAD /version?1569421336529 200 0.259 ms - 20
GET /calypso/images/favicons/favicon-development.ico 200 2.300 ms - 32956
GET /calypso/manifest.json?branch=add%2Fgm19-webauthn 200 0.410 ms - 806
GET /service-worker.js 200 0.942 ms - 2829
GET /calypso/images/manifest/icon-144x144.png?source=pwa&branch=add%2Fgm19-webauthn 200 0.796 ms - 16232
GET /calypso/images/gridicons-84d04a83ed8c3cfc40de995e9bd32649.svg 404 3.406 ms - 4167
GET /calypso/images/gridicons-84d04a83ed8c3cfc40de995e9bd32649.svg 404 2.865 ms - 4167
GET / 200 117.525 ms - 33418
GET /calypso/images/gridicons-84d04a83ed8c3cfc40de995e9bd32649.svg 404 7.377 ms - 4167
GET /calypso/manifest.json?branch=add%2Fgm19-webauthn 200 0.314 ms - 806
GET /calypso/images/gridicons-84d04a83ed8c3cfc40de995e9bd32649.svg 404 10.132 ms - 4167
GET /service-worker.js 200 57.923 ms - 2829
GET /calypso/evergreen/14eda83353fb1d1e1aee.hot-update.json 404 3.991 ms - 4167
GET /calypso/images/reader/reader-welcome-illustration.svg 200 0.540 ms - 41474
GET /calypso/images/manifest/icon-144x144.png?source=pwa&branch=add%2Fgm19-webauthn 200 0.697 ms - 16232
HEAD /version?1569421346124 200 0.483 ms - 20
GET /calypso/images/gridicons-84d04a83ed8c3cfc40de995e9bd32649.svg 404 2.829 ms - 4167
GET /calypso/images/material-icons-8d3ffc349dfae3338d60d7e89e0753a9.svg 404 7.960 ms - 4167
GET /calypso/images/material-icons-8d3ffc349dfae3338d60d7e89e0753a9.svg 404 3.414 ms - 4167
GET /calypso/images/gridicons-84d04a83ed8c3cfc40de995e9bd32649.svg 404 3.666 ms - 4167
GET /calypso/images/gridicons-84d04a83ed8c3cfc40de995e9bd32649.svg 404 2.120 ms - 4167

Reverting this change seems to fix this issue.

sirreal · 2019-09-25T19:44:37Z

Thanks @xknown, this did introduce a regression in development via npm start. I'll work on a fix.

The fix for SSR file-loader URLs from #36204 caused a regression in development. This was due to the way `publicPath` is hard-coded in the server's webpack-dev-middleware. When in that environment, use matching output and public path.

* Output file-loader files in development * Fix file-loader assets in development The fix for SSR file-loader URLs from #36204 caused a regression in development. This was due to the way `publicPath` is hard-coded in the server's webpack-dev-middleware. When in that environment, use matching output and public path. * Only emitFile once * Fix the URLs of SSR-ed images by configuring the server file-loader, too

Improve file-loader for multiple build output

79fa5a1

Output files once to images and share the output across builds.

sirreal self-assigned this Sep 20, 2019

sirreal added [Type] Bug When a feature is broken and / or not performing as intended Design Server Side Rendering labels Sep 20, 2019

sirreal added 2 commits September 20, 2019 09:45

Fix publicPath

82dce67

Fix outputPath

ab6dfa5

sirreal requested review from sgomes, michaeldcain and a team September 20, 2019 10:11

sirreal added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Sep 20, 2019

sirreal marked this pull request as ready for review September 20, 2019 10:11

sirreal commented Sep 20, 2019

View reviewed changes

webpack.config.node.js Outdated Show resolved Hide resolved

Remove extra empty line

3075e35

michaeldcain requested a review from a team September 25, 2019 02:27

p-jackson approved these changes Sep 25, 2019

View reviewed changes

sirreal mentioned this pull request Sep 25, 2019

Build: Fix SVG errors Automattic/wp-desktop#672

Closed

sirreal added [Status] Ready to Merge and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Sep 25, 2019

sirreal merged commit 65c6731 into master Sep 25, 2019

sirreal deleted the update/fix-ssr-files branch September 25, 2019 10:15

matticbot removed the [Status] Ready to Merge label Sep 25, 2019

kwight mentioned this pull request Sep 25, 2019

Gutenberg: Update e2e tests for v6.5.0. #36142

Merged

sirreal mentioned this pull request Sep 25, 2019

File-loader: Output file-loader files in development #36337

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SSR: Fix file-loader assets #36204

SSR: Fix file-loader assets #36204

sirreal commented Sep 20, 2019 •

edited

Loading

matticbot commented Sep 20, 2019

matticbot commented Sep 23, 2019 •

edited

Loading

p-jackson left a comment

xknown commented Sep 25, 2019

sirreal commented Sep 25, 2019

SSR: Fix file-loader assets #36204

SSR: Fix file-loader assets #36204

Conversation

sirreal commented Sep 20, 2019 • edited Loading

Screens

Before

SSR

Rendered

After

SSR

Rendered

Testing instructions

matticbot commented Sep 20, 2019

matticbot commented Sep 23, 2019 • edited Loading

p-jackson left a comment

Choose a reason for hiding this comment

xknown commented Sep 25, 2019

sirreal commented Sep 25, 2019

sirreal commented Sep 20, 2019 •

edited

Loading

matticbot commented Sep 23, 2019 •

edited

Loading