Skip to content

Commit

Permalink
[bundle optimization] Update to semver 7.x to get tree-shaking (#83020)…
Browse files Browse the repository at this point in the history
… (#83542)

## What's changed in this PR
### Update to latest available `semver`: `7.3.2`
 * `semver` 5.x pulls in the entire library in one large file (~38k uncompressed / ~9k gz), when we might only use 1-2K.
 * `semver` 7.0+ supports tree-shaking: https://github.com/npm/node-semver/blob/master/CHANGELOG.md#700

### Update paths to only import individual function(s) used instead of the entire library
  * Getting the smaller bundle requires a different import style [as shown in the docs](https://github.com/npm/node-semver#usage)
  * Only changed code in `public` & `common` folders; not `server`. We could also update `server` as well for consistency, but I skipped because the new import style is more verbose and the filesize didn't seem as important on the server

### Results
The build stats show a 10K+ improvement for initial page bundles #83020 (comment)

| id | [before](c6afc47) | [after](213bb52) | diff |
| --- | --- | --- | --- |
| `ingestManager` | 386.2KB | 373.9KB | -12.3KB |
| `telemetry` | 63.5KB | 50.1KB | -13.5KB |
| `upgradeAssistant` | 74.5KB | 60.5KB | -14.0KB |
| total |  |  | -39.7KB |

### The import paths look odd. Are they required?
I agree and, no, they're not strictly required. If you'd like me to revert to the prior style just drop a comment and I'll undo them.

The caveat is that the current style (in `master` & this PR) pulls in the entire `semver` library. In 7.x that added ~15K to the initial size. Some more details in the comments: #83020 (comment)

### Possible issues
Moving 2 major versions. We're currently on 5.7 and the latest available is 7.3. 
  * changelog says 5.x (our current) to 6.0 should be safe: https://github.com/npm/node-semver/blob/master/CHANGELOG.md#60
  * There 6.x & 7.x changes all appear to be new features or bugfixes around the `includePrerelease` flag added in 5.6, but I'm not sure if those "fixes" will break existing code
    * https://github.com/npm/node-semver/blob/master/CHANGELOG.md#613
    * https://github.com/npm/node-semver/blob/master/CHANGELOG.md#722

### Stats / screenshots
generated with `node scripts/build_kibana_platform_plugins.js --profile --focus=ingestManager`
<details><summary><b>Ingest Manager in `master`</b>: imports entire `semver` lib, totals 40k+, only 1 large file (orange arc below)</summary>

<img width="972" alt="Screen Shot 2020-11-09 at 6 50 23 PM" src="https://user-images.githubusercontent.com/57655/98666188-a50ac380-231a-11eb-9b8a-6ca784752714.png">
</details>

<details><summary><b>Ingest Manager in PR after upgrade to 7</b>: still imports entire lib. file size *increased* to ~60k, but now individual files are imported (orange arcs below)</summary>
<img width="825" alt="Screen Shot 2020-11-09 at 5 46 30 PM" src="https://user-images.githubusercontent.com/57655/98666355-e602d800-231a-11eb-803f-bc04beb4eaf1.png">
<img width="963" alt="Screen Shot 2020-11-09 at 5 47 06 PM" src="https://user-images.githubusercontent.com/57655/98666357-e69b6e80-231a-11eb-92d3-c66904f92c30.png">
</details>

<details><summary><b>Ingest Manager in PR after changing `import`s:</b> total imported size down to ~20k. Can see individual imported files</summary>
<img width="926" alt="Screen Shot 2020-11-10 at 6 10 23 AM" src="https://user-images.githubusercontent.com/57655/98667058-e64fa300-231b-11eb-9690-5e36ed6475e0.png">
<img width="895" alt="Screen Shot 2020-11-10 at 6 10 53 AM" src="https://user-images.githubusercontent.com/57655/98667059-e780d000-231b-11eb-8abf-98d8bdbcf061.png">
</details>

### Checklist

- [ ] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
  • Loading branch information
John Schulz authored Nov 17, 2020
1 parent c858019 commit b2a7efe
Show file tree
Hide file tree
Showing 17 changed files with 2,551 additions and 1,059 deletions.
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@
"rison-node": "1.0.2",
"rxjs": "^6.5.5",
"seedrandom": "^3.0.5",
"semver": "^5.7.0",
"semver": "^7.3.2",
"set-value": "^3.0.2",
"source-map-support": "^0.5.19",
"squel": "^5.13.0",
Expand Down Expand Up @@ -533,7 +533,7 @@
"@types/request": "^2.48.2",
"@types/seedrandom": ">=2.0.0 <4.0.0",
"@types/selenium-webdriver": "^4.0.9",
"@types/semver": "^5.5.0",
"@types/semver": "^7",
"@types/set-value": "^2.0.0",
"@types/sinon": "^7.0.13",
"@types/source-map-support": "^0.5.3",
Expand Down
3,507 changes: 2,498 additions & 1,009 deletions packages/kbn-pm/dist/index.js

Large diffs are not rendered by default.

12 changes: 6 additions & 6 deletions src/plugins/dashboard/common/migrate_to_730_panels.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
* under the License.
*/
import { i18n } from '@kbn/i18n';
import semver from 'semver';
import semverSatisfies from 'semver/functions/satisfies';
import uuid from 'uuid';
import {
GridData,
Expand Down Expand Up @@ -60,23 +60,23 @@ function isPre61Panel(
}

function is61Panel(panel: unknown | RawSavedDashboardPanel610): panel is RawSavedDashboardPanel610 {
return semver.satisfies((panel as RawSavedDashboardPanel610).version, '6.1.x');
return semverSatisfies((panel as RawSavedDashboardPanel610).version, '6.1.x');
}

function is62Panel(panel: unknown | RawSavedDashboardPanel620): panel is RawSavedDashboardPanel620 {
return semver.satisfies((panel as RawSavedDashboardPanel620).version, '6.2.x');
return semverSatisfies((panel as RawSavedDashboardPanel620).version, '6.2.x');
}

function is63Panel(panel: unknown | RawSavedDashboardPanel630): panel is RawSavedDashboardPanel630 {
return semver.satisfies((panel as RawSavedDashboardPanel630).version, '6.3.x');
return semverSatisfies((panel as RawSavedDashboardPanel630).version, '6.3.x');
}

function is640To720Panel(
panel: unknown | RawSavedDashboardPanel640To720
): panel is RawSavedDashboardPanel640To720 {
return (
semver.satisfies((panel as RawSavedDashboardPanel630).version, '>6.3') &&
semver.satisfies((panel as RawSavedDashboardPanel630).version, '<7.3')
semverSatisfies((panel as RawSavedDashboardPanel630).version, '>6.3') &&
semverSatisfies((panel as RawSavedDashboardPanel630).version, '<7.3')
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
* under the License.
*/

import semver from 'semver';
import semverSatisfies from 'semver/functions/satisfies';
import { i18n } from '@kbn/i18n';
import { METRIC_TYPE } from '@kbn/analytics';

Expand Down Expand Up @@ -68,7 +68,7 @@ export function migrateAppState(
usageCollection.reportUiStats('DashboardPanelVersionInUrl', METRIC_TYPE.LOADED, `${version}`);
}

return semver.satisfies(version, '<7.3');
return semverSatisfies(version, '<7.3');
});

if (panelNeedsMigration) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@
* under the License.
*/

import semver from 'semver';
import SemVer from 'semver/classes/semver';
import semverParse from 'semver/functions/parse';
import { TelemetrySavedObject } from './types';

interface GetTelemetryOptInConfig {
Expand Down Expand Up @@ -80,10 +81,10 @@ export const getTelemetryOptIn: GetTelemetryOptIn = ({
return savedOptIn;
};

function parseSemver(version: string): semver.SemVer | null {
function parseSemver(version: string): SemVer | null {
// semver functions both return nulls AND throw exceptions: "it depends!"
try {
return semver.parse(version);
return semverParse(version);
} catch (err) {
return null;
}
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/enterprise_search/common/version.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { SemVer } from 'semver';
import SemVer from 'semver/classes/semver';
import pkg from '../../../../package.json';

export const CURRENT_VERSION = new SemVer(pkg.version as string);
Expand Down
9 changes: 5 additions & 4 deletions x-pack/plugins/fleet/common/services/is_agent_upgradeable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
import semver from 'semver';
import semverCoerce from 'semver/functions/coerce';
import semverLt from 'semver/functions/lt';
import { Agent } from '../types';

export function isAgentUpgradeable(agent: Agent, kibanaVersion: string) {
Expand All @@ -17,9 +18,9 @@ export function isAgentUpgradeable(agent: Agent, kibanaVersion: string) {
if (!agent.local_metadata.elastic.agent.upgradeable) return false;

// make sure versions are only the number before comparison
const agentVersionNumber = semver.coerce(agentVersion);
const agentVersionNumber = semverCoerce(agentVersion);
if (!agentVersionNumber) throw new Error('agent version is invalid');
const kibanaVersionNumber = semver.coerce(kibanaVersion);
const kibanaVersionNumber = semverCoerce(kibanaVersion);
if (!kibanaVersionNumber) throw new Error('kibana version is invalid');
return semver.lt(agentVersionNumber, kibanaVersionNumber);
return semverLt(agentVersionNumber, kibanaVersionNumber);
}
6 changes: 3 additions & 3 deletions x-pack/plugins/fleet/server/routes/agent/upgrade_handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

import { RequestHandler } from 'src/core/server';
import { TypeOf } from '@kbn/config-schema';
import semver from 'semver';
import semverCoerce from 'semver/functions/coerce';
import {
AgentSOAttributes,
PostAgentUpgradeResponse,
Expand Down Expand Up @@ -122,9 +122,9 @@ export const postBulkAgentsUpgradeHandler: RequestHandler<

export const checkVersionIsSame = (version: string, kibanaVersion: string) => {
// get version number only in case "-SNAPSHOT" is in it
const kibanaVersionNumber = semver.coerce(kibanaVersion)?.version;
const kibanaVersionNumber = semverCoerce(kibanaVersion)?.version;
if (!kibanaVersionNumber) throw new Error(`kibanaVersion ${kibanaVersionNumber} is not valid`);
const versionToUpgradeNumber = semver.coerce(version)?.version;
const versionToUpgradeNumber = semverCoerce(version)?.version;
if (!versionToUpgradeNumber)
throw new Error(`version to upgrade ${versionToUpgradeNumber} is not valid`);
// temporarily only allow upgrading to the same version as the installed kibana version
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
* you may not use this file except in compliance with the Elastic License.
*/

import semver from 'semver';
import semverParse from 'semver/functions/parse';
import semverLt from 'semver/functions/lt';

import { timer, from, Observable, TimeoutError, of, EMPTY } from 'rxjs';
import { omit } from 'lodash';
import {
Expand Down Expand Up @@ -132,18 +134,14 @@ export async function createAgentActionFromPolicyAction(
policyAction: AgentPolicyAction
) {
// Transform the policy action for agent version <= 7.9.x for BWC
const agentVersion = semver.parse((agent.local_metadata?.elastic as any)?.agent?.version);
const agentVersion = semverParse((agent.local_metadata?.elastic as any)?.agent?.version);
const agentPolicyAction: AgentPolicyAction | AgentPolicyActionV7_9 =
agentVersion &&
semver.lt(
semverLt(
agentVersion,
// A prerelease tag is added here so that agent versions with prerelease tags can be compared
// correctly using `semvar`
'7.10.0-SNAPSHOT',
// `@types/semvar` is out of date with the version of `semvar` we use and doesn't have a
// corresponding release version we can update the typing to :( so, the typing error is
// suppressed here even though it is supported by `semvar`
// @ts-expect-error
{ includePrerelease: true }
)
? {
Expand Down
13 changes: 8 additions & 5 deletions x-pack/plugins/fleet/server/services/agents/enroll.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@
*/

import Boom from '@hapi/boom';
import semver from 'semver';
import semverParse from 'semver/functions/parse';
import semverDiff from 'semver/functions/diff';
import semverLte from 'semver/functions/lte';

import { SavedObjectsClientContract } from 'src/core/server';
import { AgentType, Agent, AgentSOAttributes } from '../../types';
import { savedObjectToAgent } from './saved_objects';
Expand Down Expand Up @@ -94,17 +97,17 @@ export function validateAgentVersion(
agentVersion: string,
kibanaVersion = appContextService.getKibanaVersion()
) {
const agentVersionParsed = semver.parse(agentVersion);
const agentVersionParsed = semverParse(agentVersion);
if (!agentVersionParsed) {
throw Boom.badRequest('Agent version not provided');
}

const kibanaVersionParsed = semver.parse(kibanaVersion);
const kibanaVersionParsed = semverParse(kibanaVersion);
if (!kibanaVersionParsed) {
throw Boom.badRequest('Kibana version is not set or provided');
}

const diff = semver.diff(agentVersion, kibanaVersion);
const diff = semverDiff(agentVersion, kibanaVersion);
switch (diff) {
// section 1) very close versions, only patch release differences - all combos should work
// Agent a.b.1 < Kibana a.b.2
Expand All @@ -130,7 +133,7 @@ export function validateAgentVersion(
// Agent 7.10.x > Kibana 7.9.x
// Agent 8.0.x > Kibana 7.9.x
default:
if (semver.lte(agentVersionParsed, kibanaVersionParsed)) return;
if (semverLte(agentVersionParsed, kibanaVersionParsed)) return;
else
throw Boom.badRequest(
`Agent version ${agentVersion} is not compatible with Kibana version ${kibanaVersion}`
Expand Down
7 changes: 4 additions & 3 deletions x-pack/plugins/fleet/server/services/epm/packages/install.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
* you may not use this file except in compliance with the Elastic License.
*/

import semver from 'semver';
import semverGt from 'semver/functions/gt';
import semverLt from 'semver/functions/lt';
import Boom from '@hapi/boom';
import { UnwrapPromise } from '@kbn/utility-types';
import { SavedObject, SavedObjectsClientContract } from 'src/core/server';
Expand Down Expand Up @@ -185,7 +186,7 @@ export async function upgradePackage({
latestPkg,
pkgToUpgrade,
}: UpgradePackageParams): Promise<BulkInstallResponse> {
if (!installedPkg || semver.gt(latestPkg.version, installedPkg.attributes.version)) {
if (!installedPkg || semverGt(latestPkg.version, installedPkg.attributes.version)) {
const pkgkey = Registry.pkgToPkgKey({
name: latestPkg.name,
version: latestPkg.version,
Expand Down Expand Up @@ -255,7 +256,7 @@ async function installPackageFromRegistry({
// let the user install if using the force flag or needing to reinstall or install a previous version due to failed update
const installOutOfDateVersionOk =
installType === 'reinstall' || installType === 'reupdate' || installType === 'rollback';
if (semver.lt(pkgVersion, latestPackage.version) && !force && !installOutOfDateVersionOk) {
if (semverLt(pkgVersion, latestPackage.version) && !force && !installOutOfDateVersionOk) {
throw new PackageOutdatedError(`${pkgkey} is out-of-date and cannot be installed or updated`);
}

Expand Down
4 changes: 2 additions & 2 deletions x-pack/plugins/fleet/server/services/epm/registry/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/
import mime from 'mime-types';
import semver from 'semver';
import semverValid from 'semver/functions/valid';
import { Response } from 'node-fetch';
import { URL } from 'url';
import {
Expand Down Expand Up @@ -52,7 +52,7 @@ export function splitPkgKey(pkgkey: string): { pkgName: string; pkgVersion: stri

// this will return the entire string if `indexOf` return -1
const pkgVersion = pkgkey.substr(pkgkey.indexOf('-') + 1);
if (!semver.valid(pkgVersion)) {
if (!semverValid(pkgVersion)) {
throw new Error('Package key parsing failed: package version was not a valid semver');
}
return { pkgName, pkgVersion };
Expand Down
5 changes: 2 additions & 3 deletions x-pack/plugins/ml/common/util/job_utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@
*/

import { isEmpty, isEqual, each, pick } from 'lodash';

import semver from 'semver';
import semverGte from 'semver/functions/gte';
import moment, { Duration } from 'moment';
// @ts-ignore
import numeral from '@elastic/numeral';
Expand Down Expand Up @@ -205,7 +204,7 @@ export function isModelPlotEnabled(
// created with) is greater than or equal to the supplied version (e.g. '6.1.0').
export function isJobVersionGte(job: CombinedJob, version: string): boolean {
const jobVersion = job.job_version ?? '0.0.0';
return semver.gte(jobVersion, version);
return semverGte(jobVersion, version);
}

// Takes an ML detector 'function' and returns the corresponding ES aggregation name
Expand Down
4 changes: 2 additions & 2 deletions x-pack/plugins/monitoring/public/lib/logstash/pipelines.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@
* you may not use this file except in compliance with the Elastic License.
*/

import semver from 'semver';
import semverMajor from 'semver/functions/major';
import { LOGSTASH } from '../../../common/constants';

export function isPipelineMonitoringSupportedInVersion(logstashVersion) {
const major = semver.major(logstashVersion);
const major = semverMajor(logstashVersion);
return major >= LOGSTASH.MAJOR_VER_REQD_FOR_PIPELINES;
}
2 changes: 1 addition & 1 deletion x-pack/plugins/upgrade_assistant/common/version.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { SemVer } from 'semver';
import SemVer from 'semver/classes/semver';
import pkg from '../../../../package.json';

export const CURRENT_VERSION = new SemVer(pkg.version as string);
Expand Down
4 changes: 2 additions & 2 deletions x-pack/tasks/helpers/pkg.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/

import Fs from 'fs';
import semver from 'semver';
import semverValid from 'semver/functions/valid';

interface PackageJson {
name: string;
Expand All @@ -28,6 +28,6 @@ if (!PKG_NAME) {
throw new Error('No "name" found in package.json');
}

if (!semver.valid(PKG_VERSION)) {
if (!semverValid(PKG_VERSION)) {
throw new Error(`Version is not valid semver: ${PKG_VERSION}`);
}
8 changes: 4 additions & 4 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -5705,10 +5705,10 @@
resolved "https://registry.yarnpkg.com/@types/selenium-webdriver/-/selenium-webdriver-4.0.9.tgz#12621e55b2ef8f6c98bd17fe23fa720c6cba16bd"
integrity sha512-HopIwBE7GUXsscmt/J0DhnFXLSmO04AfxT6b8HAprknwka7pqEWquWDMXxCjd+NUHK9MkCe1SDKKsMiNmCItbQ==

"@types/semver@^5.5.0":
version "5.5.0"
resolved "https://registry.yarnpkg.com/@types/semver/-/semver-5.5.0.tgz#146c2a29ee7d3bae4bf2fcb274636e264c813c45"
integrity sha512-41qEJgBH/TWgo5NFSvBCJ1qkoi3Q6ONSF2avrHq1LVEZfYpdHmj0y9SuTK+u9ZhG1sYQKBL1AWXKyLWP4RaUoQ==
"@types/semver@^7":
version "7.3.4"
resolved "https://registry.yarnpkg.com/@types/semver/-/semver-7.3.4.tgz#43d7168fec6fa0988bb1a513a697b29296721afb"
integrity sha512-+nVsLKlcUCeMzD2ufHEYuJ9a2ovstb6Dp52A5VsoKxDXgvE051XgHI/33I1EymwkRGQkwnA0LkhnUzituGs4EQ==

"@types/set-value@^2.0.0":
version "2.0.0"
Expand Down

0 comments on commit b2a7efe

Please sign in to comment.