[kbn/optimizer] fix ui/* url rewrites in dist (#58627)

* [kbn/optimizer] fix ui/* url rewrites in dist

* add tests to verify styles are built correctly and ui-rewrites are happening

* clarify change to dirs creation

* create tested & shared parsePath helper

* update renovate config

* split implementation of parsePath for dir and file paths

* switch to valid css property

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

package.json

@@ -349,6 +349,7 @@
     "@types/mustache": "^0.8.31",
     "@types/node": "^10.12.27",
     "@types/node-forge": "^0.9.0",
+    "@types/normalize-path": "^3.0.0",
     "@types/numeral": "^0.0.26",
     "@types/opn": "^5.1.0",
     "@types/pegjs": "^0.10.1",

packages/kbn-optimizer/src/__fixtures__/mock_repo/plugins/bar/public/index.ts

@@ -17,6 +17,7 @@
  * under the License.
  */
 
+import './legacy/styles.scss';
 import { fooLibFn } from '../../foo/public/index';
 export * from './lib';
 export { fooLibFn };

packages/kbn-optimizer/src/__fixtures__/mock_repo/plugins/bar/public/legacy/styles.scss

@@ -0,0 +1,4 @@
+body {
+  width: $globalStyleConstant;
+  background-image: url("ui/icon.svg");
+}

packages/kbn-optimizer/src/__fixtures__/mock_repo/src/legacy/ui/public/icon.svg

@@ -0,0 +1 @@
+<svg xmlns="http://www.w3.org/2000/svg" width="100" height="100" version="1.1"><circle cx="50" cy="50" r="25"/></svg>

packages/kbn-optimizer/src/__fixtures__/mock_repo/src/legacy/ui/public/styles/_styling_constants.scss

@@ -0,0 +1 @@
+$globalStyleConstant: 10;

packages/kbn-optimizer/src/integration_tests/basic_optimization.test.ts

@@ -24,14 +24,14 @@ import { inspect } from 'util';
 import cpy from 'cpy';
 import del from 'del';
 import { toArray, tap } from 'rxjs/operators';
-import { createAbsolutePathSerializer } from '@kbn/dev-utils';
-import { runOptimizer, OptimizerConfig, OptimizerUpdate } from '@kbn/optimizer';
+import { createAbsolutePathSerializer, ToolingLog, REPO_ROOT } from '@kbn/dev-utils';
+import { runOptimizer, OptimizerConfig, OptimizerUpdate, logOptimizerState } from '@kbn/optimizer';
 
 const TMP_DIR = Path.resolve(__dirname, '../__fixtures__/__tmp__');
 const MOCK_REPO_SRC = Path.resolve(__dirname, '../__fixtures__/mock_repo');
 const MOCK_REPO_DIR = Path.resolve(TMP_DIR, 'mock_repo');
 
-expect.addSnapshotSerializer(createAbsolutePathSerializer(MOCK_REPO_DIR));
+expect.addSnapshotSerializer(createAbsolutePathSerializer(REPO_ROOT));
 
 beforeAll(async () => {
   await del(TMP_DIR);
@@ -51,20 +51,25 @@ it('builds expected bundles, saves bundle counts to metadata', async () => {
     repoRoot: MOCK_REPO_DIR,
     pluginScanDirs: [Path.resolve(MOCK_REPO_DIR, 'plugins')],
     maxWorkerCount: 1,
+    dist: true,
   });
 
   expect(config).toMatchSnapshot('OptimizerConfig');
 
-  const msgs = await runOptimizer(config)
-    .pipe(
-      tap(state => {
-        if (state.event?.type === 'worker stdio') {
-          // eslint-disable-next-line no-console
-          console.log('worker', state.event.stream, state.event.chunk.toString('utf8'));
+  const log = new ToolingLog({
+    level: 'error',
+    writeTo: {
+      write(chunk) {
+        if (chunk.endsWith('\n')) {
+          chunk = chunk.slice(0, -1);
         }
-      }),
-      toArray()
-    )
+        // eslint-disable-next-line no-console
+        console.error(chunk);
+      },
+    },
+  });
+  const msgs = await runOptimizer(config)
+    .pipe(logOptimizerState(log, config), toArray())
     .toPromise();
 
   const assert = (statement: string, truth: boolean, altStates?: OptimizerUpdate[]) => {
@@ -133,23 +138,31 @@ it('builds expected bundles, saves bundle counts to metadata', async () => {
   expect(foo.cache.getModuleCount()).toBe(3);
   expect(foo.cache.getReferencedFiles()).toMatchInlineSnapshot(`
     Array [
-      <absolute path>/plugins/foo/public/ext.ts,
-      <absolute path>/plugins/foo/public/index.ts,
-      <absolute path>/plugins/foo/public/lib.ts,
+      <absolute path>/packages/kbn-optimizer/src/__fixtures__/__tmp__/mock_repo/plugins/foo/public/ext.ts,
+      <absolute path>/packages/kbn-optimizer/src/__fixtures__/__tmp__/mock_repo/plugins/foo/public/index.ts,
+      <absolute path>/packages/kbn-optimizer/src/__fixtures__/__tmp__/mock_repo/plugins/foo/public/lib.ts,
     ]
   `);
 
   const bar = config.bundles.find(b => b.id === 'bar')!;
   expect(bar).toBeTruthy();
   bar.cache.refresh();
-  expect(bar.cache.getModuleCount()).toBe(5);
+  expect(bar.cache.getModuleCount()).toBe(
+    // code + styles + style/css-loader runtime
+    14
+  );
+
   expect(bar.cache.getReferencedFiles()).toMatchInlineSnapshot(`
     Array [
-      <absolute path>/plugins/foo/public/ext.ts,
-      <absolute path>/plugins/foo/public/index.ts,
-      <absolute path>/plugins/foo/public/lib.ts,
-      <absolute path>/plugins/bar/public/index.ts,
-      <absolute path>/plugins/bar/public/lib.ts,
+      <absolute path>/node_modules/css-loader/package.json,
+      <absolute path>/node_modules/style-loader/package.json,
+      <absolute path>/packages/kbn-optimizer/src/__fixtures__/__tmp__/mock_repo/plugins/bar/public/index.ts,
+      <absolute path>/packages/kbn-optimizer/src/__fixtures__/__tmp__/mock_repo/plugins/bar/public/legacy/styles.scss,
+      <absolute path>/packages/kbn-optimizer/src/__fixtures__/__tmp__/mock_repo/plugins/bar/public/lib.ts,
+      <absolute path>/packages/kbn-optimizer/src/__fixtures__/__tmp__/mock_repo/plugins/foo/public/ext.ts,
+      <absolute path>/packages/kbn-optimizer/src/__fixtures__/__tmp__/mock_repo/plugins/foo/public/index.ts,
+      <absolute path>/packages/kbn-optimizer/src/__fixtures__/__tmp__/mock_repo/plugins/foo/public/lib.ts,
+      <absolute path>/packages/kbn-optimizer/src/__fixtures__/__tmp__/mock_repo/src/legacy/ui/public/icon.svg,
     ]
   `);
 });
@@ -159,6 +172,7 @@ it('uses cache on second run and exist cleanly', async () => {
     repoRoot: MOCK_REPO_DIR,
     pluginScanDirs: [Path.resolve(MOCK_REPO_DIR, 'plugins')],
     maxWorkerCount: 1,
+    dist: true,
   });
 
   const msgs = await runOptimizer(config)

packages/kbn-optimizer/src/worker/__snapshots__/parse_path.test.ts.snap

@@ -0,0 +1,156 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`parseDirPath() parses / 1`] = `
+Object {
+  "dirs": Array [],
+  "filename": undefined,
+  "root": "/",
+}
+`;
+
+exports[`parseDirPath() parses /foo 1`] = `
+Object {
+  "dirs": Array [
+    "foo",
+  ],
+  "filename": undefined,
+  "root": "/",
+}
+`;
+
+exports[`parseDirPath() parses /foo/bar/baz 1`] = `
+Object {
+  "dirs": Array [
+    "foo",
+    "bar",
+    "baz",
+  ],
+  "filename": undefined,
+  "root": "/",
+}
+`;
+
+exports[`parseDirPath() parses /foo/bar/baz/ 1`] = `
+Object {
+  "dirs": Array [
+    "foo",
+    "bar",
+    "baz",
+  ],
+  "filename": undefined,
+  "root": "/",
+}
+`;
+
+exports[`parseDirPath() parses c:\\ 1`] = `
+Object {
+  "dirs": Array [],
+  "filename": undefined,
+  "root": "c:",
+}
+`;
+
+exports[`parseDirPath() parses c:\\foo 1`] = `
+Object {
+  "dirs": Array [
+    "foo",
+  ],
+  "filename": undefined,
+  "root": "c:",
+}
+`;
+
+exports[`parseDirPath() parses c:\\foo\\bar\\baz 1`] = `
+Object {
+  "dirs": Array [
+    "foo",
+    "bar",
+    "baz",
+  ],
+  "filename": undefined,
+  "root": "c:",
+}
+`;
+
+exports[`parseDirPath() parses c:\\foo\\bar\\baz\\ 1`] = `
+Object {
+  "dirs": Array [
+    "foo",
+    "bar",
+    "baz",
+  ],
+  "filename": undefined,
+  "root": "c:",
+}
+`;
+
+exports[`parseFilePath() parses /foo 1`] = `
+Object {
+  "dirs": Array [],
+  "filename": "foo",
+  "root": "/",
+}
+`;
+
+exports[`parseFilePath() parses /foo/bar/baz 1`] = `
+Object {
+  "dirs": Array [
+    "foo",
+    "bar",
+  ],
+  "filename": "baz",
+  "root": "/",
+}
+`;
+
+exports[`parseFilePath() parses /foo/bar/baz.json 1`] = `
+Object {
+  "dirs": Array [
+    "foo",
+    "bar",
+  ],
+  "filename": "baz.json",
+  "root": "/",
+}
+`;
+
+exports[`parseFilePath() parses c:/foo/bar/baz.json 1`] = `
+Object {
+  "dirs": Array [
+    "foo",
+    "bar",
+  ],
+  "filename": "baz.json",
+  "root": "c:",
+}
+`;
+
+exports[`parseFilePath() parses c:\\foo 1`] = `
+Object {
+  "dirs": Array [],
+  "filename": "foo",
+  "root": "c:",
+}
+`;
+
+exports[`parseFilePath() parses c:\\foo\\bar\\baz 1`] = `
+Object {
+  "dirs": Array [
+    "foo",
+    "bar",
+  ],
+  "filename": "baz",
+  "root": "c:",
+}
+`;
+
+exports[`parseFilePath() parses c:\\foo\\bar\\baz.json 1`] = `
+Object {
+  "dirs": Array [
+    "foo",
+    "bar",
+  ],
+  "filename": "baz.json",
+  "root": "c:",
+}
+`;

packages/kbn-optimizer/src/worker/parse_path.test.ts

@@ -17,8 +17,20 @@
  * under the License.
  */
 
-declare function NormalizePath(path: string, stripTrailing?: boolean): string;
+import { parseFilePath, parseDirPath } from './parse_path';
 
-declare module 'normalize-path' {
-  export = NormalizePath;
-}
+const DIRS = ['/', '/foo/bar/baz/', 'c:\\', 'c:\\foo\\bar\\baz\\'];
+const AMBIGUOUS = ['/foo', '/foo/bar/baz', 'c:\\foo', 'c:\\foo\\bar\\baz'];
+const FILES = ['/foo/bar/baz.json', 'c:/foo/bar/baz.json', 'c:\\foo\\bar\\baz.json'];
+
+describe('parseFilePath()', () => {
+  it.each([...FILES, ...AMBIGUOUS])('parses %s', path => {
+    expect(parseFilePath(path)).toMatchSnapshot();
+  });
+});
+
+describe('parseDirPath()', () => {
+  it.each([...DIRS, ...AMBIGUOUS])('parses %s', path => {
+    expect(parseDirPath(path)).toMatchSnapshot();
+  });
+});

packages/kbn-optimizer/src/worker/parse_path.ts

@@ -0,0 +1,43 @@
+/*
+ * Licensed to Elasticsearch B.V. under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch B.V. licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import normalizePath from 'normalize-path';
+
+/**
+ * Parse an absolute path, supporting normalized paths from webpack,
+ * into a list of directories and root
+ */
+export function parseDirPath(path: string) {
+  const filePath = parseFilePath(path);
+  return {
+    ...filePath,
+    dirs: [...filePath.dirs, ...(filePath.filename ? [filePath.filename] : [])],
+    filename: undefined,
+  };
+}
+
+export function parseFilePath(path: string) {
+  const normalized = normalizePath(path);
+  const [root, ...others] = normalized.split('/');
+  return {
+    root: root === '' ? '/' : root,
+    dirs: others.slice(0, -1),
+    filename: others[others.length - 1] || undefined,
+  };
+}

packages/kbn-optimizer/src/worker/run_compilers.ts

@@ -27,9 +27,10 @@ import webpack, { Stats } from 'webpack';
 import * as Rx from 'rxjs';
 import { mergeMap, map, mapTo, takeUntil } from 'rxjs/operators';
 
-import { CompilerMsgs, CompilerMsg, maybeMap, Bundle, WorkerConfig } from '../common';
+import { CompilerMsgs, CompilerMsg, maybeMap, Bundle, WorkerConfig, ascending } from '../common';
 import { getWebpackConfig } from './webpack.config';
 import { isFailureStats, failedStatsToErrorMessage } from './webpack_helpers';
+import { parseFilePath } from './parse_path';
 import {
   isExternalModule,
   isNormalModule,
@@ -108,20 +109,19 @@ const observeCompiler = (
 
       for (const module of normalModules) {
         const path = getModulePath(module);
+        const parsedPath = parseFilePath(path);
 
-        const parsedPath = Path.parse(path);
-        const dirSegments = parsedPath.dir.split(Path.sep);
-        if (!dirSegments.includes('node_modules')) {
+        if (!parsedPath.dirs.includes('node_modules')) {
           referencedFiles.add(path);
           continue;
         }
 
-        const nmIndex = dirSegments.lastIndexOf('node_modules');
-        const isScoped = dirSegments[nmIndex + 1].startsWith('@');
+        const nmIndex = parsedPath.dirs.lastIndexOf('node_modules');
+        const isScoped = parsedPath.dirs[nmIndex + 1].startsWith('@');
         referencedFiles.add(
           Path.join(
             parsedPath.root,
-            ...dirSegments.slice(0, nmIndex + 1 + (isScoped ? 2 : 1)),
+            ...parsedPath.dirs.slice(0, nmIndex + 1 + (isScoped ? 2 : 1)),
             'package.json'
           )
         );
@@ -146,7 +146,7 @@ const observeCompiler = (
         optimizerCacheKey: workerConfig.optimizerCacheKey,
         cacheKey: bundle.createCacheKey(files, mtimes),
         moduleCount: normalModules.length,
-        files,
+        files: files.sort(ascending(f => f)),
       });
 
       return compilerMsgs.compilerSuccess({

packages/kbn-optimizer/src/worker/webpack.config.ts

@@ -30,6 +30,7 @@ import { CleanWebpackPlugin } from 'clean-webpack-plugin';
 import * as SharedDeps from '@kbn/ui-shared-deps';
 
 import { Bundle, WorkerConfig } from '../common';
+import { parseDirPath } from './parse_path';
 
 const IS_CODE_COVERAGE = !!process.env.CODE_COVERAGE;
 const ISTANBUL_PRESET_PATH = require.resolve('@kbn/babel-preset/istanbul_preset');
@@ -135,7 +136,7 @@ export function getWebpackConfig(bundle: Bundle, worker: WorkerConfig) {
                       }
 
                       // manually force ui/* urls in legacy styles to resolve to ui/legacy/public
-                      if (uri.startsWith('ui/') && base.split(Path.sep).includes('legacy')) {
+                      if (uri.startsWith('ui/') && parseDirPath(base).dirs.includes('legacy')) {
                         return Path.resolve(
                           worker.repoRoot,
                           'src/legacy/ui/public',
@@ -150,7 +151,9 @@ export function getWebpackConfig(bundle: Bundle, worker: WorkerConfig) {
                 {
                   loader: 'sass-loader',
                   options: {
-                    sourceMap: !worker.dist,
+                    // must always be enabled as long as we're using the `resolve-url-loader` to
+                    // rewrite `ui/*` urls. They're dropped by subsequent loaders though
+                    sourceMap: true,
                     prependData(loaderContext: webpack.loader.LoaderContext) {
                       return `@import ${stringifyRequest(
                         loaderContext,

renovate.json5

@@ -665,6 +665,14 @@
           '@types/nodemailer',
         ],
       },
+      {
+        groupSlug: 'normalize-path',
+        groupName: 'normalize-path related packages',
+        packageNames: [
+          'normalize-path',
+          '@types/normalize-path',
+        ],
+      },
       {
         groupSlug: 'numeral',
         groupName: 'numeral related packages',

yarn.lock

@@ -4864,6 +4864,11 @@
   resolved "https://registry.yarnpkg.com/@types/normalize-package-data/-/normalize-package-data-2.4.0.tgz#e486d0d97396d79beedd0a6e33f4534ff6b4973e"
   integrity sha512-f5j5b/Gf71L+dbqxIpQ4Z2WlmI/mPJ0fOkGGmFgtb6sAu97EPczzbS3/tJKxmcYDj55OX6ssqwDAWOHIYDRDGA==
 
+"@types/normalize-path@^3.0.0":
+  version "3.0.0"
+  resolved "https://registry.yarnpkg.com/@types/normalize-path/-/normalize-path-3.0.0.tgz#bb5c46cab77b93350b4cf8d7ff1153f47189ae31"
+  integrity sha512-Nd8y/5t/7CRakPYiyPzr/IAfYusy1FkcZYFEAcoMZkwpJv2n4Wm+olW+e7xBdHEXhOnWdG9ddbar0gqZWS4x5Q==
+
 "@types/numeral@^0.0.25":
   version "0.0.25"
   resolved "https://registry.yarnpkg.com/@types/numeral/-/numeral-0.0.25.tgz#b6f55062827a4787fe4ab151cf3412a468e65271"