Skip to content

Webpack config in dev and prod include copywebpack for static transla… #1491

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Jul 13, 2020
Merged

Webpack config in dev and prod include copywebpack for static transla… #1491

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
78f87b6
Webpack config in dev and prod include copywebpack for static transla…
oruburos Jul 10, 2020
348056e
Webpack config in dev and prod include copywebpack for static transla…
oruburos Jul 10, 2020
c31a65a
Webpack config in dev and prod include copywebpack for static transla…
oruburos Jul 10, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion client/i18n.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ const fallbackLng = ['en-US'];
const availableLanguages = ['en-US', 'es-419'];

const options = {
loadPath: '/translations/{{lng}}/translations.json',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should stay as '/translations/{{lng}}/translations.json', but the static file config in server/server.js needs to change...

loadPath: 'locales/{{lng}}/translations.json',
requestOptions: { // used for fetch, can also be a function (payload) => ({ method: 'GET' })
mode: 'no-cors'
},
Expand Down
1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@
"connect-mongo": "^1.3.2",
"console-feed": "^2.8.11",
"cookie-parser": "^1.4.3",
"copy-webpack-plugin": "^6.0.3",
"cors": "^2.8.5",
"cross-env": "^5.2.1",
"csslint": "^1.0.5",
Expand Down
4 changes: 2 additions & 2 deletions server/server.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,11 @@ app.use(corsMiddleware);
app.options('*', corsMiddleware);

// Body parser, cookie parser, sessions, serve public assets

app.use('/translations', Express.static('translations/locales/'));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The path needs to point to the location of the copied translations...

app.use('/translations', Express.static(path.resolve(__dirname, '../dist/static/translations'));

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andrewn given your suggested fix here, I think this line may be unnecessary, given L79, which serves everything in /dist/static to /, which I think would include subfolders. Definitely double check and test this though!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@catarak, you're right that L79 will serve /dist/static/translations but it will sent with a max-age of 1 day. Everything bundled by WebPack uses the file's content hash in the filename (e.g. app.c41b13c86f9f5577f9cc.js). The browser can cache this for a long time (in principle, forever) since it'll never change. If the bundle's updated, it has a new filename and the browser fetches the file at the new URL.

For now, we're not versioning the translations assets using a content hash in the filename so we should serve them from /dist/static/translations but with a different cache strategy. Using an E-Tag with no max-age means that the browser can make a quick conditional GET request to check if the file's been updated. If it hasn't been updated, no data is received but if it has, it will be sent the latest file.

In future we should probably use the same content-hash-in-filename strategy for these files too, but it's a bit more complex so this is a fix in the short-term that gets staging working again.

@oruburos this line (L78) needs to be:

app.use(
  '/locales',
  Express.static(
    path.resolve(__dirname, '../dist/static/locales'),
    {
      // Browsers must revalidate for changes to the locale files
      // It doesn't actually mean "don't cache this file"
      // See: https://jakearchibald.com/2016/caching-best-practices/
      setHeaders: res => res.setHeader('Cache-Control', 'no-cache')
    }
  )
);

app.use(Express.static(path.resolve(__dirname, '../dist/static'), {
maxAge: process.env.STATIC_MAX_AGE || (process.env.NODE_ENV === 'production' ? '1d' : '0')
}));
app.use('/translations', Express.static('translations/locales/'));

app.use(bodyParser.urlencoded({ limit: '50mb', extended: true }));
app.use(bodyParser.json({ limit: '50mb' }));
app.use(cookieParser());
Expand Down
9 changes: 8 additions & 1 deletion webpack/config.dev.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
const webpack = require('webpack');
const path = require('path');
const CopyWebpackPlugin = require('copy-webpack-plugin')

if (process.env.NODE_ENV === 'development') {
require('dotenv').config();
Expand Down Expand Up @@ -40,7 +41,13 @@ module.exports = {
'process.env': {
NODE_ENV: JSON.stringify('development')
}
})
}),
new CopyWebpackPlugin({
patterns: [
{from: path.resolve(__dirname, '../translations/locales') , to: path.resolve(__dirname, 'locales')}
]
}
)
],
module: {
rules: [
Expand Down
9 changes: 8 additions & 1 deletion webpack/config.prod.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ const cssnext = require('postcss-cssnext');
const postcssFocus = require('postcss-focus');
const postcssReporter = require('postcss-reporter');
const cssnano = require('cssnano');
const CopyWebpackPlugin = require('copy-webpack-plugin')
if (process.env.NODE_ENV === "development") {
require('dotenv').config();
}
Expand Down Expand Up @@ -144,7 +145,13 @@ module.exports = [{
}),
new MiniCssExtractPlugin({
filename: 'app.[hash].css',
})
}),
new CopyWebpackPlugin({
patterns: [
{from: path.resolve(__dirname, '../translations/locales') , to: path.resolve(__dirname, '../dist/static/locales')}
]
}
)
]
},
{
Expand Down