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

[7.x] Do not serve legacy JS when serving a Kibana Platform applicat… (#61011) #64218

Merged
merged 1 commit into from
Apr 22, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 9 additions & 1 deletion packages/kbn-optimizer/src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,11 @@ run(
throw createFlagError('expected --cache to have no value');
}

const includeCoreBundle = flags.core ?? true;
if (typeof includeCoreBundle !== 'boolean') {
throw createFlagError('expected --core to have no value');
}

const dist = flags.dist ?? false;
if (typeof dist !== 'boolean') {
throw createFlagError('expected --dist to have no value');
Expand Down Expand Up @@ -87,6 +92,7 @@ run(
profileWebpack,
extraPluginScanDirs,
inspectWorkers,
includeCoreBundle,
});

await runOptimizer(config)
Expand All @@ -95,9 +101,10 @@ run(
},
{
flags: {
boolean: ['watch', 'oss', 'examples', 'dist', 'cache', 'profile', 'inspect-workers'],
boolean: ['core', 'watch', 'oss', 'examples', 'dist', 'cache', 'profile', 'inspect-workers'],
string: ['workers', 'scan-dir'],
default: {
core: true,
examples: true,
cache: true,
'inspect-workers': true,
Expand All @@ -107,6 +114,7 @@ run(
--workers max number of workers to use
--oss only build oss plugins
--profile profile the webpack builds and write stats.json files to build outputs
--no-core disable generating the core bundle
--no-cache disable the cache
--no-examples don't build the example plugins
--dist create bundles that are suitable for inclusion in the Kibana distributable
Expand Down
2 changes: 1 addition & 1 deletion packages/kbn-optimizer/src/common/bundle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import { BundleCache } from './bundle_cache';
import { UnknownVals } from './ts_helpers';
import { includes, ascending, entriesToObject } from './array_helpers';

const VALID_BUNDLE_TYPES = ['plugin' as const];
const VALID_BUNDLE_TYPES = ['plugin' as const, 'entry' as const];

export interface BundleSpec {
readonly type: typeof VALID_BUNDLE_TYPES[0];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,13 @@

import { createAbsolutePathSerializer } from '@kbn/dev-utils';

import { getBundles } from './get_bundles';
import { getPluginBundles } from './get_plugin_bundles';

expect.addSnapshotSerializer(createAbsolutePathSerializer('/repo'));

it('returns a bundle for each plugin', () => {
it('returns a bundle for core and each plugin', () => {
expect(
getBundles(
getPluginBundles(
[
{
directory: '/repo/plugins/foo',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import { Bundle } from '../common';

import { KibanaPlatformPlugin } from './kibana_platform_plugins';

export function getBundles(plugins: KibanaPlatformPlugin[], repoRoot: string) {
export function getPluginBundles(plugins: KibanaPlatformPlugin[], repoRoot: string) {
return plugins
.filter(p => p.isUiPlugin)
.map(
Expand Down
28 changes: 22 additions & 6 deletions packages/kbn-optimizer/src/optimizer/optimizer_config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

jest.mock('./assign_bundles_to_workers.ts');
jest.mock('./kibana_platform_plugins.ts');
jest.mock('./get_bundles.ts');
jest.mock('./get_plugin_bundles.ts');

import Path from 'path';
import Os from 'os';
Expand Down Expand Up @@ -90,6 +90,7 @@ describe('OptimizerConfig::parseOptions()', () => {
Object {
"cache": true,
"dist": false,
"includeCoreBundle": false,
"inspectWorkers": false,
"maxWorkerCount": 2,
"pluginPaths": Array [],
Expand All @@ -114,6 +115,7 @@ describe('OptimizerConfig::parseOptions()', () => {
Object {
"cache": false,
"dist": false,
"includeCoreBundle": false,
"inspectWorkers": false,
"maxWorkerCount": 2,
"pluginPaths": Array [],
Expand All @@ -138,6 +140,7 @@ describe('OptimizerConfig::parseOptions()', () => {
Object {
"cache": true,
"dist": false,
"includeCoreBundle": false,
"inspectWorkers": false,
"maxWorkerCount": 2,
"pluginPaths": Array [],
Expand All @@ -164,6 +167,7 @@ describe('OptimizerConfig::parseOptions()', () => {
Object {
"cache": true,
"dist": false,
"includeCoreBundle": false,
"inspectWorkers": false,
"maxWorkerCount": 2,
"pluginPaths": Array [],
Expand All @@ -187,6 +191,7 @@ describe('OptimizerConfig::parseOptions()', () => {
Object {
"cache": true,
"dist": false,
"includeCoreBundle": false,
"inspectWorkers": false,
"maxWorkerCount": 2,
"pluginPaths": Array [],
Expand All @@ -210,6 +215,7 @@ describe('OptimizerConfig::parseOptions()', () => {
Object {
"cache": true,
"dist": false,
"includeCoreBundle": false,
"inspectWorkers": false,
"maxWorkerCount": 100,
"pluginPaths": Array [],
Expand All @@ -230,6 +236,7 @@ describe('OptimizerConfig::parseOptions()', () => {
Object {
"cache": false,
"dist": false,
"includeCoreBundle": false,
"inspectWorkers": false,
"maxWorkerCount": 100,
"pluginPaths": Array [],
Expand All @@ -250,6 +257,7 @@ describe('OptimizerConfig::parseOptions()', () => {
Object {
"cache": false,
"dist": false,
"includeCoreBundle": false,
"inspectWorkers": false,
"maxWorkerCount": 100,
"pluginPaths": Array [],
Expand All @@ -271,6 +279,7 @@ describe('OptimizerConfig::parseOptions()', () => {
Object {
"cache": false,
"dist": false,
"includeCoreBundle": false,
"inspectWorkers": false,
"maxWorkerCount": 100,
"pluginPaths": Array [],
Expand All @@ -292,6 +301,7 @@ describe('OptimizerConfig::parseOptions()', () => {
Object {
"cache": true,
"dist": false,
"includeCoreBundle": false,
"inspectWorkers": false,
"maxWorkerCount": 100,
"pluginPaths": Array [],
Expand All @@ -314,7 +324,7 @@ describe('OptimizerConfig::create()', () => {
.assignBundlesToWorkers;
const findKibanaPlatformPlugins: jest.Mock = jest.requireMock('./kibana_platform_plugins.ts')
.findKibanaPlatformPlugins;
const getBundles: jest.Mock = jest.requireMock('./get_bundles.ts').getBundles;
const getPluginBundles: jest.Mock = jest.requireMock('./get_plugin_bundles.ts').getPluginBundles;

beforeEach(() => {
if ('mock' in OptimizerConfig.parseOptions) {
Expand All @@ -326,7 +336,7 @@ describe('OptimizerConfig::create()', () => {
{ config: Symbol('worker config 2') },
]);
findKibanaPlatformPlugins.mockReturnValue(Symbol('new platform plugins'));
getBundles.mockReturnValue(Symbol('bundles'));
getPluginBundles.mockReturnValue([Symbol('bundle1'), Symbol('bundle2')]);

jest.spyOn(OptimizerConfig, 'parseOptions').mockImplementation((): any => ({
cache: Symbol('parsed cache'),
Expand All @@ -348,7 +358,10 @@ describe('OptimizerConfig::create()', () => {

expect(config).toMatchInlineSnapshot(`
OptimizerConfig {
"bundles": Symbol(bundles),
"bundles": Array [
Symbol(bundle1),
Symbol(bundle2),
],
"cache": Symbol(parsed cache),
"dist": Symbol(parsed dist),
"inspectWorkers": Symbol(parsed inspect workers),
Expand Down Expand Up @@ -383,7 +396,7 @@ describe('OptimizerConfig::create()', () => {
}
`);

expect(getBundles.mock).toMatchInlineSnapshot(`
expect(getPluginBundles.mock).toMatchInlineSnapshot(`
Object {
"calls": Array [
Array [
Expand All @@ -400,7 +413,10 @@ describe('OptimizerConfig::create()', () => {
"results": Array [
Object {
"type": "return",
"value": Symbol(bundles),
"value": Array [
Symbol(bundle1),
Symbol(bundle2),
],
},
],
}
Expand Down
24 changes: 22 additions & 2 deletions packages/kbn-optimizer/src/optimizer/optimizer_config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import Os from 'os';
import { Bundle, WorkerConfig } from '../common';

import { findKibanaPlatformPlugins, KibanaPlatformPlugin } from './kibana_platform_plugins';
import { getBundles } from './get_bundles';
import { getPluginBundles } from './get_plugin_bundles';

function pickMaxWorkerCount(dist: boolean) {
// don't break if cpus() returns nothing, or an empty array
Expand Down Expand Up @@ -60,6 +60,9 @@ interface Options {
pluginScanDirs?: string[];
/** absolute paths that should be added to the default scan dirs */
extraPluginScanDirs?: string[];

/** flag that causes the core bundle to be built along with plugins */
includeCoreBundle?: boolean;
}

interface ParsedOptions {
Expand All @@ -72,6 +75,7 @@ interface ParsedOptions {
pluginPaths: string[];
pluginScanDirs: string[];
inspectWorkers: boolean;
includeCoreBundle: boolean;
}

export class OptimizerConfig {
Expand All @@ -83,6 +87,7 @@ export class OptimizerConfig {
const profileWebpack = !!options.profileWebpack;
const inspectWorkers = !!options.inspectWorkers;
const cache = options.cache !== false && !process.env.KBN_OPTIMIZER_NO_CACHE;
const includeCoreBundle = !!options.includeCoreBundle;

const repoRoot = options.repoRoot;
if (!Path.isAbsolute(repoRoot)) {
Expand Down Expand Up @@ -134,13 +139,28 @@ export class OptimizerConfig {
pluginScanDirs,
pluginPaths,
inspectWorkers,
includeCoreBundle,
};
}

static create(inputOptions: Options) {
const options = OptimizerConfig.parseOptions(inputOptions);
const plugins = findKibanaPlatformPlugins(options.pluginScanDirs, options.pluginPaths);
const bundles = getBundles(plugins, options.repoRoot);
const bundles = [
...(options.includeCoreBundle
? [
new Bundle({
type: 'entry',
id: 'core',
entry: './public/entry_point',
sourceRoot: options.repoRoot,
contextDir: Path.resolve(options.repoRoot, 'src/core'),
outputDir: Path.resolve(options.repoRoot, 'src/core/target/public'),
}),
]
: []),
...getPluginBundles(plugins, options.repoRoot),
];

return new OptimizerConfig(
bundles,
Expand Down
2 changes: 1 addition & 1 deletion packages/kbn-optimizer/src/worker/webpack.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ export function getWebpackConfig(bundle: Bundle, worker: WorkerConfig) {

output: {
path: bundle.outputDir,
filename: '[name].plugin.js',
filename: `[name].${bundle.type}.js`,
publicPath: PUBLIC_PATH_PLACEHOLDER,
devtoolModuleFilenameTemplate: info =>
`/${bundle.type}:${bundle.id}/${Path.relative(
Expand Down
1 change: 1 addition & 0 deletions src/cli/cluster/run_kbn_optimizer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ export function runKbnOptimizer(opts: Record<string, any>, config: LegacyConfig)
const optimizerConfig = OptimizerConfig.create({
repoRoot: REPO_ROOT,
watch: true,
includeCoreBundle: true,
oss: !!opts.oss,
examples: !!opts.runExamples,
pluginPaths: config.get('plugins.paths'),
Expand Down
File renamed without changes.
2 changes: 1 addition & 1 deletion src/core/public/application/ui/app_router.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import React, { FunctionComponent, useMemo } from 'react';
import { Route, RouteComponentProps, Router, Switch } from 'react-router-dom';
import { History } from 'history';
import { Observable } from 'rxjs';
import { useObservable } from 'react-use';
import useObservable from 'react-use/lib/useObservable';

import { AppLeaveHandler, AppStatus, Mounter } from '../types';
import { AppContainer } from './app_container';
Expand Down
12 changes: 7 additions & 5 deletions src/core/public/core_system.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ const defaultCoreSystemParams = {
warnLegacyBrowsers: true,
},
} as any,
requireLegacyFiles: jest.fn(),
};

beforeEach(() => {
Expand Down Expand Up @@ -104,19 +103,22 @@ describe('constructor', () => {
});
});

it('passes requireLegacyFiles, useLegacyTestHarness, and a dom element to LegacyPlatformService', () => {
it('passes required params to LegacyPlatformService', () => {
const requireLegacyFiles = { requireLegacyFiles: true };
const useLegacyTestHarness = { useLegacyTestHarness: true };
const requireLegacyBootstrapModule = { requireLegacyBootstrapModule: true };
const requireNewPlatformShimModule = { requireNewPlatformShimModule: true };

createCoreSystem({
requireLegacyFiles,
useLegacyTestHarness,
requireLegacyBootstrapModule,
requireNewPlatformShimModule,
});

expect(LegacyPlatformServiceConstructor).toHaveBeenCalledTimes(1);
expect(LegacyPlatformServiceConstructor).toHaveBeenCalledWith({
requireLegacyFiles,
useLegacyTestHarness,
requireLegacyBootstrapModule,
requireNewPlatformShimModule,
});
});

Expand Down
13 changes: 7 additions & 6 deletions src/core/public/core_system.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@
* under the License.
*/

import './core.css';

import { CoreId } from '../server';
import { PackageInfo, EnvironmentMode } from '../server/types';
import { CoreSetup, CoreStart } from '.';
Expand Down Expand Up @@ -50,8 +48,9 @@ interface Params {
rootDomElement: HTMLElement;
browserSupportsCsp: boolean;
injectedMetadata: InjectedMetadataParams['injectedMetadata'];
requireLegacyFiles: LegacyPlatformParams['requireLegacyFiles'];
useLegacyTestHarness?: LegacyPlatformParams['useLegacyTestHarness'];
requireLegacyFiles?: LegacyPlatformParams['requireLegacyFiles'];
requireLegacyBootstrapModule?: LegacyPlatformParams['requireLegacyBootstrapModule'];
requireNewPlatformShimModule?: LegacyPlatformParams['requireNewPlatformShimModule'];
}

/** @internal */
Expand Down Expand Up @@ -111,7 +110,8 @@ export class CoreSystem {
browserSupportsCsp,
injectedMetadata,
requireLegacyFiles,
useLegacyTestHarness,
requireLegacyBootstrapModule,
requireNewPlatformShimModule,
} = params;

this.rootDomElement = rootDomElement;
Expand Down Expand Up @@ -145,7 +145,8 @@ export class CoreSystem {

this.legacy = new LegacyPlatformService({
requireLegacyFiles,
useLegacyTestHarness,
requireLegacyBootstrapModule,
requireNewPlatformShimModule,
});
}

Expand Down
Loading