From f39d2e7770ac347805c8ef947bca090c7f332413 Mon Sep 17 00:00:00 2001 From: Mickey Burks Date: Fri, 3 May 2019 13:31:49 -0700 Subject: [PATCH 01/14] Instrument promise with translation keys --- build/compiler.js | 3 +- build/get-webpack-config.js | 38 ++++++++++------ build/plugins/i18n-discovery-plugin.js | 14 ++++-- ...ented-import-dependency-template-plugin.js | 43 ++++++++++++++++--- 4 files changed, 74 insertions(+), 24 deletions(-) diff --git a/build/compiler.js b/build/compiler.js index 30b404ce..b1bf4fbd 100644 --- a/build/compiler.js +++ b/build/compiler.js @@ -153,7 +153,8 @@ function Compiler( clientChunkMetadata, legacyClientChunkMetadata, mergedClientChunkMetadata, - i18nManifest: new DeferredState(), + i18nManifest: new Map(), + i18nDeferredManifest: new DeferredState(), legacyBuildEnabled, }; const root = path.resolve(dir); diff --git a/build/get-webpack-config.js b/build/get-webpack-config.js index cb781028..3f7af2ee 100644 --- a/build/get-webpack-config.js +++ b/build/get-webpack-config.js @@ -86,7 +86,8 @@ export type WebpackConfigOpts = {| clientChunkMetadata: ClientChunkMetadataState, legacyClientChunkMetadata: ClientChunkMetadataState, mergedClientChunkMetadata: ClientChunkMetadataState, - i18nManifest: TranslationsManifestState, + i18nManifest: Map>, + i18nDeferredManifest: TranslationsManifestState, legacyBuildEnabled: LegacyBuildEnabledState, }, fusionConfig: FusionRC, @@ -451,10 +452,13 @@ function getWebpackConfig(opts /*: WebpackConfigOpts */) { state.mergedClientChunkMetadata ), runtime === 'client' - ? new I18nDiscoveryPlugin(state.i18nManifest) + ? new I18nDiscoveryPlugin( + state.i18nDeferredManifest, + state.i18nManifest + ) : new LoaderContextProviderPlugin( translationsManifestContextKey, - state.i18nManifest + state.i18nDeferredManifest ), !dev && zopfli && zopfliWebpackPlugin, !dev && brotliWebpackPlugin, @@ -465,17 +469,20 @@ function getWebpackConfig(opts /*: WebpackConfigOpts */) { // in dev because the CLI will not exit with an error code if the option is enabled, // so failed builds would look like successful ones. watch && new webpack.NoEmitOnErrorsPlugin(), - new InstrumentedImportDependencyTemplatePlugin( - runtime !== 'client' - ? // Server + runtime === 'server' + ? // Server + new InstrumentedImportDependencyTemplatePlugin( state.mergedClientChunkMetadata - : /** - * Client - * Don't wait for the client manifest on the client. - * The underlying plugin handles client instrumentation on its own. - */ - void 0 - ), + ) + : /** + * Client + * Don't wait for the client manifest on the client. + * The underlying plugin handles client instrumentation on its own. + */ + new InstrumentedImportDependencyTemplatePlugin( + void 0, + state.i18nManifest + ), dev && hmr && watch && new webpack.HotModuleReplacementPlugin(), !dev && runtime === 'client' && new webpack.HashedModuleIdsPlugin(), runtime === 'client' && @@ -527,7 +534,10 @@ function getWebpackConfig(opts /*: WebpackConfigOpts */) { options.optimization.splitChunks ), // need to re-apply template - new InstrumentedImportDependencyTemplatePlugin(void 0), + new InstrumentedImportDependencyTemplatePlugin( + void 0, + state.i18nManifest + ), new ClientChunkMetadataStateHydratorPlugin( state.legacyClientChunkMetadata ), diff --git a/build/plugins/i18n-discovery-plugin.js b/build/plugins/i18n-discovery-plugin.js index ddcc22e1..8e31a7bb 100644 --- a/build/plugins/i18n-discovery-plugin.js +++ b/build/plugins/i18n-discovery-plugin.js @@ -17,11 +17,14 @@ import type {TranslationsManifestState, TranslationsManifest} from "../types.js" class I18nDiscoveryPlugin { /*:: manifest: TranslationsManifestState; - discoveryState: TranslationsManifest; + discoveryState: Map>; */ - constructor(manifest /*: TranslationsManifestState*/) { - this.manifest = manifest; - this.discoveryState = new Map(); + constructor( + deferredManifest /*: TranslationsManifestState*/, + discoveryState /*: Map>*/ + ) { + this.manifest = deferredManifest; + this.discoveryState = discoveryState; } apply(compiler /*: any */) { const name = this.constructor.name; @@ -31,6 +34,9 @@ class I18nDiscoveryPlugin { context[translationsDiscoveryKey] = this.discoveryState; }); }); + // if resolve at 'make' - discoverState will be empty + // if resolve at 'afterCompile' or 'done' - instrumentation plugin is + // waiting for this to resolve from the 'make' step - build breaks compiler.hooks.done.tap(name, () => { this.manifest.resolve(this.discoveryState); }); diff --git a/build/plugins/instrumented-import-dependency-template-plugin.js b/build/plugins/instrumented-import-dependency-template-plugin.js index 7f09e4c1..04144376 100644 --- a/build/plugins/instrumented-import-dependency-template-plugin.js +++ b/build/plugins/instrumented-import-dependency-template-plugin.js @@ -35,8 +35,9 @@ const ImportDependencyTemplate = require('webpack/lib/dependencies/ImportDepende class InstrumentedImportDependencyTemplate extends ImportDependencyTemplate { /*:: clientChunkIndex: ?$PropertyType; */ - constructor(clientChunkMetadata /*: ?ClientChunkMetadata */) { + constructor(clientChunkMetadata /*: ?ClientChunkMetadata */, discoveryState) { super(); + this.discoveryState = discoveryState; if (clientChunkMetadata) { this.clientChunkIndex = clientChunkMetadata.fileManifest; } @@ -69,13 +70,33 @@ class InstrumentedImportDependencyTemplate extends ImportDependencyTemplate { chunkIds = getChunkGroupIds(depBlock.chunkGroup); } + let translationKeys = []; + if (this.discoveryState) { + const chunks = depBlock.chunkGroup.chunks; + const modules = chunks.reduce((acc, chunk) => { + const modules = Array.from(chunk._modules.keys()); + return acc.concat(modules.map(m => m.resource)); + }, []); + + translationKeys = modules.reduce((acc, module) => { + if (this.discoveryState.has(module)) { + const keys = Array.from(this.discoveryState.get(module).keys()); + return acc.concat(keys); + } else { + return acc; + } + }, []); + } + // Add the following properties to the promise returned by import() // - `__CHUNK_IDS`: the webpack chunk ids for the dynamic import // - `__MODULE_ID`: the webpack module id of the dynamically imported module. Equivalent to require.resolveWeak(path) + // - `__I18N_KEYS`: the translation keys that are used in this bundle const customContent = chunkIds ? `Object.defineProperties(${content}, { "__CHUNK_IDS": {value:${JSON.stringify(chunkIds)}}, - "__MODULE_ID": {value:${JSON.stringify(dep.module.id)}} + "__MODULE_ID": {value:${JSON.stringify(dep.module.id)}}, + "__I18N_KEYS": {value:${JSON.stringify(translationKeys)}} })` : content; @@ -91,9 +112,14 @@ class InstrumentedImportDependencyTemplate extends ImportDependencyTemplate { class InstrumentedImportDependencyTemplatePlugin { /*:: clientChunkIndexState: ?ClientChunkMetadataState; */ + /*:: translationsDiscoveryState: ?Map>; */ - constructor(clientChunkIndexState /*: ?ClientChunkMetadataState*/) { + constructor( + clientChunkIndexState /*: ?ClientChunkMetadataState*/, + translationsDiscoveryState /*: ?Map>*/ + ) { this.clientChunkIndexState = clientChunkIndexState; + this.translationsDiscoveryState = translationsDiscoveryState; } apply(compiler /*: any */) { @@ -113,13 +139,20 @@ class InstrumentedImportDependencyTemplatePlugin { ); done(); }); - } else { + } else if (this.translationsDiscoveryState) { // client compilation.dependencyTemplates.set( ImportDependency, - new InstrumentedImportDependencyTemplate() + new InstrumentedImportDependencyTemplate( + void 0, + this.translationsDiscoveryState + ) ); done(); + } else { + throw new Error( + 'InstrumentationImportDependencyPlugin called without clientChunkIndexState or translationsDiscoveryState' + ); } }); } From adc025a2024a99dcec9a7ccbcb3691ff91838bc0 Mon Sep 17 00:00:00 2001 From: Mickey Burks Date: Fri, 3 May 2019 13:39:04 -0700 Subject: [PATCH 02/14] Fix types --- build/get-webpack-config.js | 3 +- build/plugins/i18n-discovery-plugin.js | 23 +++++++-------- ...ented-import-dependency-template-plugin.js | 29 +++++++++++-------- 3 files changed, 29 insertions(+), 26 deletions(-) diff --git a/build/get-webpack-config.js b/build/get-webpack-config.js index 3f7af2ee..7fad5635 100644 --- a/build/get-webpack-config.js +++ b/build/get-webpack-config.js @@ -65,6 +65,7 @@ const JS_EXT_PATTERN = /\.jsx?$/; /*:: import type { ClientChunkMetadataState, + TranslationsManifest, TranslationsManifestState, LegacyBuildEnabledState, } from "./types.js"; @@ -86,7 +87,7 @@ export type WebpackConfigOpts = {| clientChunkMetadata: ClientChunkMetadataState, legacyClientChunkMetadata: ClientChunkMetadataState, mergedClientChunkMetadata: ClientChunkMetadataState, - i18nManifest: Map>, + i18nManifest: TranslationManifest, i18nDeferredManifest: TranslationsManifestState, legacyBuildEnabled: LegacyBuildEnabledState, }, diff --git a/build/plugins/i18n-discovery-plugin.js b/build/plugins/i18n-discovery-plugin.js index 8e31a7bb..9c081197 100644 --- a/build/plugins/i18n-discovery-plugin.js +++ b/build/plugins/i18n-discovery-plugin.js @@ -16,33 +16,30 @@ import type {TranslationsManifestState, TranslationsManifest} from "../types.js" class I18nDiscoveryPlugin { /*:: - manifest: TranslationsManifestState; - discoveryState: Map>; + manifestState: TranslationsManifestState; + manifest: TranslationsManifest; */ constructor( - deferredManifest /*: TranslationsManifestState*/, - discoveryState /*: Map>*/ + manifestState /*: TranslationsManifestState*/, + manifest /*: TranslationsManifest*/ ) { - this.manifest = deferredManifest; - this.discoveryState = discoveryState; + this.manifestState = manifestState; + this.manifest = manifest; } apply(compiler /*: any */) { const name = this.constructor.name; // "thisCompilation" is not run in child compilations compiler.hooks.thisCompilation.tap(name, compilation => { compilation.hooks.normalModuleLoader.tap(name, (context, module) => { - context[translationsDiscoveryKey] = this.discoveryState; + context[translationsDiscoveryKey] = this.manifest; }); }); - // if resolve at 'make' - discoverState will be empty - // if resolve at 'afterCompile' or 'done' - instrumentation plugin is - // waiting for this to resolve from the 'make' step - build breaks compiler.hooks.done.tap(name, () => { - this.manifest.resolve(this.discoveryState); + this.manifestState.resolve(this.manifest); }); compiler.hooks.invalid.tap(name, filename => { - this.manifest.reset(); - this.discoveryState.delete(filename); + this.manifestState.reset(); + this.manifest.delete(filename); }); } } diff --git a/build/plugins/instrumented-import-dependency-template-plugin.js b/build/plugins/instrumented-import-dependency-template-plugin.js index 04144376..a46305d8 100644 --- a/build/plugins/instrumented-import-dependency-template-plugin.js +++ b/build/plugins/instrumented-import-dependency-template-plugin.js @@ -9,7 +9,11 @@ /* eslint-env node */ /*:: -import type {ClientChunkMetadataState, ClientChunkMetadata} from "../types.js"; +import type { + ClientChunkMetadataState, + ClientChunkMetadata, + TranslationsManifest, +} from "../types.js"; */ const ImportDependency = require('webpack/lib/dependencies/ImportDependency'); @@ -34,10 +38,11 @@ const ImportDependencyTemplate = require('webpack/lib/dependencies/ImportDepende class InstrumentedImportDependencyTemplate extends ImportDependencyTemplate { /*:: clientChunkIndex: ?$PropertyType; */ + /*:: manifest: ?TranslationsManifest; */ - constructor(clientChunkMetadata /*: ?ClientChunkMetadata */, discoveryState) { + constructor(clientChunkMetadata /*: ?ClientChunkMetadata */, translationsManifest /*: ?TranslationsManifest*/) { super(); - this.discoveryState = discoveryState; + this.translationsManifest = translationsManifest; if (clientChunkMetadata) { this.clientChunkIndex = clientChunkMetadata.fileManifest; } @@ -71,7 +76,7 @@ class InstrumentedImportDependencyTemplate extends ImportDependencyTemplate { } let translationKeys = []; - if (this.discoveryState) { + if (this.translationsManifest) { const chunks = depBlock.chunkGroup.chunks; const modules = chunks.reduce((acc, chunk) => { const modules = Array.from(chunk._modules.keys()); @@ -79,8 +84,8 @@ class InstrumentedImportDependencyTemplate extends ImportDependencyTemplate { }, []); translationKeys = modules.reduce((acc, module) => { - if (this.discoveryState.has(module)) { - const keys = Array.from(this.discoveryState.get(module).keys()); + if (this.translationsManifest.has(module)) { + const keys = Array.from(this.translationsManifest.get(module).keys()); return acc.concat(keys); } else { return acc; @@ -112,14 +117,14 @@ class InstrumentedImportDependencyTemplate extends ImportDependencyTemplate { class InstrumentedImportDependencyTemplatePlugin { /*:: clientChunkIndexState: ?ClientChunkMetadataState; */ - /*:: translationsDiscoveryState: ?Map>; */ + /*:: translationsManifest: ?TranslationsManifest; */ constructor( clientChunkIndexState /*: ?ClientChunkMetadataState*/, - translationsDiscoveryState /*: ?Map>*/ + translationsManifest /*: ?TranslationsManifest*/ ) { this.clientChunkIndexState = clientChunkIndexState; - this.translationsDiscoveryState = translationsDiscoveryState; + this.translationsManifest = translationsManifest; } apply(compiler /*: any */) { @@ -139,19 +144,19 @@ class InstrumentedImportDependencyTemplatePlugin { ); done(); }); - } else if (this.translationsDiscoveryState) { + } else if (this.translationsManifest) { // client compilation.dependencyTemplates.set( ImportDependency, new InstrumentedImportDependencyTemplate( void 0, - this.translationsDiscoveryState + this.translationsManifest ) ); done(); } else { throw new Error( - 'InstrumentationImportDependencyPlugin called without clientChunkIndexState or translationsDiscoveryState' + 'InstrumentationImportDependencyPlugin called without clientChunkIndexState or translationsManifest' ); } }); From d7cc0e12a1af382dc5162e727a4cc65525236a18 Mon Sep 17 00:00:00 2001 From: Mickey Burks Date: Fri, 3 May 2019 13:40:14 -0700 Subject: [PATCH 03/14] Lint and flow --- build/get-webpack-config.js | 2 +- .../instrumented-import-dependency-template-plugin.js | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/build/get-webpack-config.js b/build/get-webpack-config.js index 7fad5635..987a62e0 100644 --- a/build/get-webpack-config.js +++ b/build/get-webpack-config.js @@ -87,7 +87,7 @@ export type WebpackConfigOpts = {| clientChunkMetadata: ClientChunkMetadataState, legacyClientChunkMetadata: ClientChunkMetadataState, mergedClientChunkMetadata: ClientChunkMetadataState, - i18nManifest: TranslationManifest, + i18nManifest: TranslationsManifest, i18nDeferredManifest: TranslationsManifestState, legacyBuildEnabled: LegacyBuildEnabledState, }, diff --git a/build/plugins/instrumented-import-dependency-template-plugin.js b/build/plugins/instrumented-import-dependency-template-plugin.js index a46305d8..7b08e22e 100644 --- a/build/plugins/instrumented-import-dependency-template-plugin.js +++ b/build/plugins/instrumented-import-dependency-template-plugin.js @@ -40,7 +40,10 @@ class InstrumentedImportDependencyTemplate extends ImportDependencyTemplate { /*:: clientChunkIndex: ?$PropertyType; */ /*:: manifest: ?TranslationsManifest; */ - constructor(clientChunkMetadata /*: ?ClientChunkMetadata */, translationsManifest /*: ?TranslationsManifest*/) { + constructor( + clientChunkMetadata /*: ?ClientChunkMetadata */, + translationsManifest /*: ?TranslationsManifest*/ + ) { super(); this.translationsManifest = translationsManifest; if (clientChunkMetadata) { From 14fe62ffae692ebad0fac60a7682b0255b888294 Mon Sep 17 00:00:00 2001 From: Mickey Burks Date: Fri, 3 May 2019 14:03:08 -0700 Subject: [PATCH 04/14] Fix broken test --- test/e2e/transpile-node-modules/test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/transpile-node-modules/test.js b/test/e2e/transpile-node-modules/test.js index 525094c5..ba1cb1ac 100644 --- a/test/e2e/transpile-node-modules/test.js +++ b/test/e2e/transpile-node-modules/test.js @@ -55,5 +55,5 @@ test('transpiles node_modules', async () => { ], }); - t.ok(clientVendor.includes(`'fixturepkg_string'`)); + t.ok(clientVendor.includes(`"fixturepkg_string"`)); }, 100000); From 7236f0200614996a389c2d75b729501d7bbf18c5 Mon Sep 17 00:00:00 2001 From: Mickey Burks Date: Tue, 7 May 2019 17:39:58 -0700 Subject: [PATCH 05/14] Fix bug where translation keys were not added in production build --- ...ented-import-dependency-template-plugin.js | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/build/plugins/instrumented-import-dependency-template-plugin.js b/build/plugins/instrumented-import-dependency-template-plugin.js index 7b08e22e..7ad8ec90 100644 --- a/build/plugins/instrumented-import-dependency-template-plugin.js +++ b/build/plugins/instrumented-import-dependency-template-plugin.js @@ -80,12 +80,24 @@ class InstrumentedImportDependencyTemplate extends ImportDependencyTemplate { let translationKeys = []; if (this.translationsManifest) { - const chunks = depBlock.chunkGroup.chunks; - const modules = chunks.reduce((acc, chunk) => { + const modulesSet = new Set(); + + // Module dependencies + if (dep.module && dep.module.dependencies) { + dep.module.dependencies.map(d => { + if (d.originModule) { + modulesSet.add(d.originModule.userRequest); + } + }); + } + + // Chunks + depBlock.chunkGroup.chunks.forEach(chunk => { const modules = Array.from(chunk._modules.keys()); - return acc.concat(modules.map(m => m.resource)); - }, []); + modules.forEach(m => modulesSet.add(m.resource)); + }); + const modules = Array.from(modulesSet.keys()); translationKeys = modules.reduce((acc, module) => { if (this.translationsManifest.has(module)) { const keys = Array.from(this.translationsManifest.get(module).keys()); From af8fb96e4c7457e47ca6d29c458ee81ee8d653d7 Mon Sep 17 00:00:00 2001 From: Mickey Burks Date: Tue, 7 May 2019 17:40:22 -0700 Subject: [PATCH 06/14] Add tests for translation keys instrumentation --- .../fixture/src/main.js | 24 +++++++++++ .../fixture/src/split-child.js | 10 +++++ .../fixture/src/split-with-child.js | 12 ++++++ .../fixture/src/split.js | 10 +++++ .../fixture/translations/en-US.json | 5 +++ test/e2e/dynamic-import-translations/test.js | 43 +++++++++++++++++++ 6 files changed, 104 insertions(+) create mode 100644 test/e2e/dynamic-import-translations/fixture/src/main.js create mode 100644 test/e2e/dynamic-import-translations/fixture/src/split-child.js create mode 100644 test/e2e/dynamic-import-translations/fixture/src/split-with-child.js create mode 100644 test/e2e/dynamic-import-translations/fixture/src/split.js create mode 100644 test/e2e/dynamic-import-translations/fixture/translations/en-US.json create mode 100644 test/e2e/dynamic-import-translations/test.js diff --git a/test/e2e/dynamic-import-translations/fixture/src/main.js b/test/e2e/dynamic-import-translations/fixture/src/main.js new file mode 100644 index 00000000..f5b810f6 --- /dev/null +++ b/test/e2e/dynamic-import-translations/fixture/src/main.js @@ -0,0 +1,24 @@ +// @noflow + +import React from 'react'; +import App from 'fusion-react'; + +function Root () { + const split = import('./split.js'); + const splitWithChild = import('./split-with-child.js'); + return ( +
+
+ {JSON.stringify(split.__I18N_KEYS)} +
+
+ {JSON.stringify(splitWithChild.__I18N_KEYS)} +
+
+ ); +} + +export default async function start() { + const app = new App(); + return app; +} diff --git a/test/e2e/dynamic-import-translations/fixture/src/split-child.js b/test/e2e/dynamic-import-translations/fixture/src/split-child.js new file mode 100644 index 00000000..313ab5f3 --- /dev/null +++ b/test/e2e/dynamic-import-translations/fixture/src/split-child.js @@ -0,0 +1,10 @@ +// @noflow + +import React from 'react'; +import {Translate} from 'fusion-plugin-i18n-react'; + +export default function SplitRouteChild() { + return ( + + ); +} diff --git a/test/e2e/dynamic-import-translations/fixture/src/split-with-child.js b/test/e2e/dynamic-import-translations/fixture/src/split-with-child.js new file mode 100644 index 00000000..7740f286 --- /dev/null +++ b/test/e2e/dynamic-import-translations/fixture/src/split-with-child.js @@ -0,0 +1,12 @@ +// @noflow + +import React, {Component} from 'react'; +import {withTranslations} from 'fusion-plugin-i18n-react'; + +import SplitRouteChild from './split-child.js'; + +function SplitRouteWithChild () { + return ; +} + +export default withTranslations(['__SPLIT_WITH_CHILD__'])(SplitRouteWithChild); diff --git a/test/e2e/dynamic-import-translations/fixture/src/split.js b/test/e2e/dynamic-import-translations/fixture/src/split.js new file mode 100644 index 00000000..ebcc7d93 --- /dev/null +++ b/test/e2e/dynamic-import-translations/fixture/src/split.js @@ -0,0 +1,10 @@ +// @noflow + +import React, {Component} from 'react'; +import {withTranslations} from 'fusion-plugin-i18n-react'; + +function SplitRoute () { + return
+} + +export default withTranslations(['__SPLIT__'])(SplitRoute); diff --git a/test/e2e/dynamic-import-translations/fixture/translations/en-US.json b/test/e2e/dynamic-import-translations/fixture/translations/en-US.json new file mode 100644 index 00000000..5b8ac225 --- /dev/null +++ b/test/e2e/dynamic-import-translations/fixture/translations/en-US.json @@ -0,0 +1,5 @@ +{ + "__SPLIT__": "", + "__SPLIT_WITH_CHILD__": "", + "__SPLIT_CHILD__": "" +} diff --git a/test/e2e/dynamic-import-translations/test.js b/test/e2e/dynamic-import-translations/test.js new file mode 100644 index 00000000..9776c1e0 --- /dev/null +++ b/test/e2e/dynamic-import-translations/test.js @@ -0,0 +1,43 @@ +// @flow +/* eslint-env node */ + +const t = require('assert'); +const path = require('path'); +const fs = require('fs'); +const puppeteer = require('puppeteer'); + +const dev = require('../setup.js'); + +const {cmd, start} = require('../utils.js'); + +const dir = path.resolve(__dirname, './fixture'); + +test('`fusion build` app with split translations integration', async () => { + var env = Object.create(process.env); + env.NODE_ENV = 'production'; + + await cmd(`build --dir=${dir} --production`, {env}); + + const {proc, port} = await start(`--dir=${dir}`, {env, cwd: dir}); + const browser = await puppeteer.launch({ + args: ['--no-sandbox', '--disable-setuid-sandbox'], + }); + const page = await browser.newPage(); + await page.goto(`http://localhost:${port}/`, {waitUntil: 'load'}); + const content = await page.content(); + t.ok( + content.includes('
["__SPLIT__"]
'), + 'translation keys are added to promise instrumentation' + ); + t.ok( + content.includes( + '
' + + '["__SPLIT_CHILD__","__SPLIT_WITH_CHILD__"]' + + '
' + ), + 'translation keys contain keys from child imports' + ); + + browser.close(); + proc.kill(); +}, 100000); From 130dbe6d4cb66576fddac84542795022653b9409 Mon Sep 17 00:00:00 2001 From: Mickey Burks Date: Wed, 8 May 2019 09:35:14 -0700 Subject: [PATCH 07/14] Fix lint --- test/e2e/dynamic-import-translations/test.js | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/test/e2e/dynamic-import-translations/test.js b/test/e2e/dynamic-import-translations/test.js index 9776c1e0..681abcff 100644 --- a/test/e2e/dynamic-import-translations/test.js +++ b/test/e2e/dynamic-import-translations/test.js @@ -3,11 +3,8 @@ const t = require('assert'); const path = require('path'); -const fs = require('fs'); const puppeteer = require('puppeteer'); -const dev = require('../setup.js'); - const {cmd, start} = require('../utils.js'); const dir = path.resolve(__dirname, './fixture'); @@ -32,8 +29,8 @@ test('`fusion build` app with split translations integration', async () => { t.ok( content.includes( '
' + - '["__SPLIT_CHILD__","__SPLIT_WITH_CHILD__"]' + - '
' + '["__SPLIT_CHILD__","__SPLIT_WITH_CHILD__"]' + + '
' ), 'translation keys contain keys from child imports' ); From c17ab5755f76ac61637faacbd0e931e323d4a463 Mon Sep 17 00:00:00 2001 From: Mickey Burks Date: Wed, 8 May 2019 10:27:57 -0700 Subject: [PATCH 08/14] Fix tests --- test/e2e/assets/test.js | 2 +- test/e2e/empty/test.js | 2 +- test/e2e/noop-test/test.js | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/test/e2e/assets/test.js b/test/e2e/assets/test.js index 46c2faf6..87f5b6e7 100644 --- a/test/e2e/assets/test.js +++ b/test/e2e/assets/test.js @@ -68,7 +68,7 @@ test('`fusion dev` works with assets', async () => { const clientMain = await request(`${url}/_static/client-main.js`); t.ok(clientMain, 'serves client-main from memory correctly'); t.ok( - clientMain.includes('"src", "src/main.js")'), + clientMain.includes('"src","src/main.js")'), 'transpiles __dirname and __filename' ); t.ok( diff --git a/test/e2e/empty/test.js b/test/e2e/empty/test.js index 957cec72..b3e0a6ff 100644 --- a/test/e2e/empty/test.js +++ b/test/e2e/empty/test.js @@ -26,7 +26,7 @@ test('generates error if missing default export', async () => { // $FlowFixMe t.fail('did not error'); } catch (e) { - t.ok(e.stderr.includes('initialize is not a function')); + t.ok(e.stderr.includes(' is not a function')); } finally { proc.kill(); } diff --git a/test/e2e/noop-test/test.js b/test/e2e/noop-test/test.js index af7a773c..bdb42aed 100644 --- a/test/e2e/noop-test/test.js +++ b/test/e2e/noop-test/test.js @@ -31,11 +31,11 @@ test('development env globals', async () => { const clientContent = await readFile(clientEntryPath, 'utf8'); t.ok( - clientContent.includes(`'main __BROWSER__ is', true`), + clientContent.includes(`"main __BROWSER__ is",!0`), `__BROWSER__ is transpiled to be true in development` ); t.ok( - clientContent.includes(`'main __NODE__ is', false`), + clientContent.includes(`"main __NODE__ is",!1`), '__NODE__ is transpiled to be false' ); From 142a2f24231e3ca766c4dec0db49d35727a14e4b Mon Sep 17 00:00:00 2001 From: Ryan Tsao Date: Wed, 8 May 2019 12:38:04 -0700 Subject: [PATCH 09/14] Update build/get-webpack-config.js Co-Authored-By: micburks --- build/get-webpack-config.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build/get-webpack-config.js b/build/get-webpack-config.js index 987a62e0..0f7d6ae0 100644 --- a/build/get-webpack-config.js +++ b/build/get-webpack-config.js @@ -478,7 +478,7 @@ function getWebpackConfig(opts /*: WebpackConfigOpts */) { : /** * Client * Don't wait for the client manifest on the client. - * The underlying plugin handles client instrumentation on its own. + * The underlying plugin is able determine client chunk metadata on its own. */ new InstrumentedImportDependencyTemplatePlugin( void 0, From 58dc4422cc8e4c3ee7974e18d4fe08ad7eb845aa Mon Sep 17 00:00:00 2001 From: Ryan Tsao Date: Wed, 8 May 2019 12:40:28 -0700 Subject: [PATCH 10/14] Update build/plugins/instrumented-import-dependency-template-plugin.js Co-Authored-By: micburks --- build/plugins/instrumented-import-dependency-template-plugin.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build/plugins/instrumented-import-dependency-template-plugin.js b/build/plugins/instrumented-import-dependency-template-plugin.js index 7ad8ec90..5fc715ba 100644 --- a/build/plugins/instrumented-import-dependency-template-plugin.js +++ b/build/plugins/instrumented-import-dependency-template-plugin.js @@ -111,7 +111,7 @@ class InstrumentedImportDependencyTemplate extends ImportDependencyTemplate { // Add the following properties to the promise returned by import() // - `__CHUNK_IDS`: the webpack chunk ids for the dynamic import // - `__MODULE_ID`: the webpack module id of the dynamically imported module. Equivalent to require.resolveWeak(path) - // - `__I18N_KEYS`: the translation keys that are used in this bundle + // - `__I18N_KEYS`: the translation keys used in the client chunk group for this import() const customContent = chunkIds ? `Object.defineProperties(${content}, { "__CHUNK_IDS": {value:${JSON.stringify(chunkIds)}}, From 039274bece6e1f2eb35b32f4fbe805e682fdde1e Mon Sep 17 00:00:00 2001 From: Mickey Burks Date: Wed, 8 May 2019 13:01:08 -0700 Subject: [PATCH 11/14] Cleanup/refactor module logic --- ...ented-import-dependency-template-plugin.js | 50 ++++++++++--------- 1 file changed, 27 insertions(+), 23 deletions(-) diff --git a/build/plugins/instrumented-import-dependency-template-plugin.js b/build/plugins/instrumented-import-dependency-template-plugin.js index 5fc715ba..93fe105c 100644 --- a/build/plugins/instrumented-import-dependency-template-plugin.js +++ b/build/plugins/instrumented-import-dependency-template-plugin.js @@ -80,32 +80,14 @@ class InstrumentedImportDependencyTemplate extends ImportDependencyTemplate { let translationKeys = []; if (this.translationsManifest) { - const modulesSet = new Set(); - - // Module dependencies - if (dep.module && dep.module.dependencies) { - dep.module.dependencies.map(d => { - if (d.originModule) { - modulesSet.add(d.originModule.userRequest); - } - }); - } + const modules = getChunkGroupModules(dep); - // Chunks - depBlock.chunkGroup.chunks.forEach(chunk => { - const modules = Array.from(chunk._modules.keys()); - modules.forEach(m => modulesSet.add(m.resource)); - }); - - const modules = Array.from(modulesSet.keys()); - translationKeys = modules.reduce((acc, module) => { + for (const module of modules) { if (this.translationsManifest.has(module)) { - const keys = Array.from(this.translationsManifest.get(module).keys()); - return acc.concat(keys); - } else { - return acc; + const keys = this.translationsManifest.get(module).keys(); + translationKeys.push(...keys); } - }, []); + } } // Add the following properties to the promise returned by import() @@ -192,3 +174,25 @@ function getChunkGroupIds(chunkGroup) { return [chunkGroup.id]; } } + +function getChunkGroupModules(dep) { + const modulesSet = new Set(); + + // Module dependencies + if (dep.module && dep.module.dependencies) { + dep.module.dependencies.forEach(dependency => { + if (dependency.originModule) { + modulesSet.add(dependency.originModule.userRequest); + } + }); + } + + // Chunks + dep.block.chunkGroup.chunks.forEach(chunk => { + for (const module of chunk._modules) { + modulesSet.add(module.resource); + } + }); + + return modulesSet; +} From 5b8b60a08ececebae3d403f541dc6eb47055552f Mon Sep 17 00:00:00 2001 From: Mickey Burks Date: Wed, 8 May 2019 14:24:24 -0700 Subject: [PATCH 12/14] lint --fix --- build/plugins/instrumented-import-dependency-template-plugin.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build/plugins/instrumented-import-dependency-template-plugin.js b/build/plugins/instrumented-import-dependency-template-plugin.js index 93fe105c..c8a28b30 100644 --- a/build/plugins/instrumented-import-dependency-template-plugin.js +++ b/build/plugins/instrumented-import-dependency-template-plugin.js @@ -93,7 +93,7 @@ class InstrumentedImportDependencyTemplate extends ImportDependencyTemplate { // Add the following properties to the promise returned by import() // - `__CHUNK_IDS`: the webpack chunk ids for the dynamic import // - `__MODULE_ID`: the webpack module id of the dynamically imported module. Equivalent to require.resolveWeak(path) - // - `__I18N_KEYS`: the translation keys used in the client chunk group for this import() + // - `__I18N_KEYS`: the translation keys used in the client chunk group for this import() const customContent = chunkIds ? `Object.defineProperties(${content}, { "__CHUNK_IDS": {value:${JSON.stringify(chunkIds)}}, From bd441674e557a45b2b54069c1644ea2d9afd20c6 Mon Sep 17 00:00:00 2001 From: Mickey Burks Date: Thu, 9 May 2019 09:53:43 -0700 Subject: [PATCH 13/14] Refactor InstrumentationPlugin parameters to be an object --- build/get-webpack-config.js | 23 ++++----- ...ented-import-dependency-template-plugin.js | 48 ++++++++++++------- 2 files changed, 42 insertions(+), 29 deletions(-) diff --git a/build/get-webpack-config.js b/build/get-webpack-config.js index 0f7d6ae0..d4dcf489 100644 --- a/build/get-webpack-config.js +++ b/build/get-webpack-config.js @@ -472,18 +472,19 @@ function getWebpackConfig(opts /*: WebpackConfigOpts */) { watch && new webpack.NoEmitOnErrorsPlugin(), runtime === 'server' ? // Server - new InstrumentedImportDependencyTemplatePlugin( - state.mergedClientChunkMetadata - ) + new InstrumentedImportDependencyTemplatePlugin({ + compilation: 'server', + clientChunkMetadata: state.mergedClientChunkMetadata, + }) : /** * Client * Don't wait for the client manifest on the client. * The underlying plugin is able determine client chunk metadata on its own. */ - new InstrumentedImportDependencyTemplatePlugin( - void 0, - state.i18nManifest - ), + new InstrumentedImportDependencyTemplatePlugin({ + compilation: 'client', + i18nManifest: state.i18nManifest, + }), dev && hmr && watch && new webpack.HotModuleReplacementPlugin(), !dev && runtime === 'client' && new webpack.HashedModuleIdsPlugin(), runtime === 'client' && @@ -535,10 +536,10 @@ function getWebpackConfig(opts /*: WebpackConfigOpts */) { options.optimization.splitChunks ), // need to re-apply template - new InstrumentedImportDependencyTemplatePlugin( - void 0, - state.i18nManifest - ), + new InstrumentedImportDependencyTemplatePlugin({ + compilation: 'client', + i18nManifest: state.i18nManifest, + }), new ClientChunkMetadataStateHydratorPlugin( state.legacyClientChunkMetadata ), diff --git a/build/plugins/instrumented-import-dependency-template-plugin.js b/build/plugins/instrumented-import-dependency-template-plugin.js index c8a28b30..b8eac487 100644 --- a/build/plugins/instrumented-import-dependency-template-plugin.js +++ b/build/plugins/instrumented-import-dependency-template-plugin.js @@ -14,6 +14,20 @@ import type { ClientChunkMetadata, TranslationsManifest, } from "../types.js"; + +type InstrumentationPluginOpts = + | ClientPluginOpts + | ServerPluginOpts; + +type ServerPluginOpts = { + compilation: "server", + clientChunkMetadata: ClientChunkMetadataState +}; + +type ClientPluginOpts = { + compilation: "client", + i18nManifest: TranslationsManifest +}; */ const ImportDependency = require('webpack/lib/dependencies/ImportDependency'); @@ -41,8 +55,10 @@ class InstrumentedImportDependencyTemplate extends ImportDependencyTemplate { /*:: manifest: ?TranslationsManifest; */ constructor( - clientChunkMetadata /*: ?ClientChunkMetadata */, - translationsManifest /*: ?TranslationsManifest*/ + { + clientChunkMetadata, + translationsManifest, + } /*: {clientChunkMetadata?: ClientChunkMetadata, translationsManifest?: TranslationsManifest}*/ ) { super(); this.translationsManifest = translationsManifest; @@ -113,15 +129,10 @@ class InstrumentedImportDependencyTemplate extends ImportDependencyTemplate { */ class InstrumentedImportDependencyTemplatePlugin { - /*:: clientChunkIndexState: ?ClientChunkMetadataState; */ - /*:: translationsManifest: ?TranslationsManifest; */ + /*:: opts: InstrumentationPluginOpts;*/ - constructor( - clientChunkIndexState /*: ?ClientChunkMetadataState*/, - translationsManifest /*: ?TranslationsManifest*/ - ) { - this.clientChunkIndexState = clientChunkIndexState; - this.translationsManifest = translationsManifest; + constructor(opts /*: InstrumentationPluginOpts*/) { + this.opts = opts; } apply(compiler /*: any */) { @@ -132,23 +143,24 @@ class InstrumentedImportDependencyTemplatePlugin { * `make` is the subsequent lifeycle method, so we can override this value here. */ compiler.hooks.make.tapAsync(name, (compilation, done) => { - if (this.clientChunkIndexState) { + if (this.opts.compilation === 'server') { // server - this.clientChunkIndexState.result.then(chunkIndex => { + this.opts.clientChunkMetadata.result.then(chunkIndex => { compilation.dependencyTemplates.set( ImportDependency, - new InstrumentedImportDependencyTemplate(chunkIndex) + new InstrumentedImportDependencyTemplate({ + clientChunkMetadata: chunkIndex, + }) ); done(); }); - } else if (this.translationsManifest) { + } else if (this.opts.compilation === 'client') { // client compilation.dependencyTemplates.set( ImportDependency, - new InstrumentedImportDependencyTemplate( - void 0, - this.translationsManifest - ) + new InstrumentedImportDependencyTemplate({ + translationsManifest: this.opts.i18nManifest, + }) ); done(); } else { From 07b757aaa1c5ffc306c7780338fa44dae0634009 Mon Sep 17 00:00:00 2001 From: Mickey Burks Date: Thu, 9 May 2019 10:06:57 -0700 Subject: [PATCH 14/14] Be more clear in comments, remove empty lines --- .../instrumented-import-dependency-template-plugin.js | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/build/plugins/instrumented-import-dependency-template-plugin.js b/build/plugins/instrumented-import-dependency-template-plugin.js index b8eac487..04bd0cf1 100644 --- a/build/plugins/instrumented-import-dependency-template-plugin.js +++ b/build/plugins/instrumented-import-dependency-template-plugin.js @@ -97,7 +97,6 @@ class InstrumentedImportDependencyTemplate extends ImportDependencyTemplate { let translationKeys = []; if (this.translationsManifest) { const modules = getChunkGroupModules(dep); - for (const module of modules) { if (this.translationsManifest.has(module)) { const keys = this.translationsManifest.get(module).keys(); @@ -189,8 +188,7 @@ function getChunkGroupIds(chunkGroup) { function getChunkGroupModules(dep) { const modulesSet = new Set(); - - // Module dependencies + // For ConcatenatedModules in production build if (dep.module && dep.module.dependencies) { dep.module.dependencies.forEach(dependency => { if (dependency.originModule) { @@ -198,13 +196,11 @@ function getChunkGroupModules(dep) { } }); } - - // Chunks + // For NormalModules dep.block.chunkGroup.chunks.forEach(chunk => { for (const module of chunk._modules) { modulesSet.add(module.resource); } }); - return modulesSet; }