fix(core): fix resolver performance regression (#2654)

wix · Aug 11, 2022 · 5ab7803 · 5ab7803
1 parent ac7103b
commit 5ab7803
Show file tree

Hide file tree

Showing 4 changed files with 59 additions and 29 deletions.
diff --git a/packages/core/src/stylable-resolver.ts b/packages/core/src/stylable-resolver.ts
@@ -48,7 +48,18 @@ export interface CachedJsModule {
     value: JsModule;
 }
 
-export type CachedModuleEntity = InvalidCachedModule | CachedStylableMeta | CachedJsModule;
+export interface ResolveOnly {
+    resolvedPath: string;
+    kind: 'resolve';
+    value: null;
+}
+
+export type CachedModuleEntity =
+    | InvalidCachedModule
+    | CachedStylableMeta
+    | CachedJsModule
+    | ResolveOnly;
+
 export type StylableResolverCache = Map<string, CachedModuleEntity>;
 
 export interface CSSResolve<T extends StylableSymbol = StylableSymbol> {
@@ -106,16 +117,22 @@ export class StylableResolver {
         protected cache?: StylableResolverCache
     ) {}
     private getModule({ context, request }: Imported): CachedModuleEntity {
+        let entity: CachedModuleEntity;
+        let resolvedPath: string | undefined;
+
         const key = cacheKey(context, request);
+
         if (this.cache?.has(key)) {
-            return this.cache.get(key)!;
+            const entity = this.cache.get(key)!;
+            if (entity.kind === 'resolve') {
+                resolvedPath = entity.resolvedPath;
+            } else {
+                return entity;
+            }
         }
 
-        let entity: CachedModuleEntity;
-        let resolvedPath: string;
-
         try {
-            resolvedPath = this.moduleResolver(context, request);
+            resolvedPath ||= this.moduleResolver(context, request);
         } catch (error) {
             entity = {
                 kind: request.endsWith('css') ? 'css' : 'js',
@@ -150,11 +167,18 @@ export class StylableResolver {
         return entity;
     }
     public resolvePath(directoryPath: string, request: string): string {
-        const resolvedPath = this.cache?.get(cacheKey(directoryPath, request))?.resolvedPath;
-        if (resolvedPath) {
+        const key = cacheKey(directoryPath, request);
+        let resolvedPath = this.cache?.get(key)?.resolvedPath;
+        if (resolvedPath !== undefined) {
             return resolvedPath;
         }
-        return this.moduleResolver(directoryPath, request);
+        resolvedPath = this.moduleResolver(directoryPath, request);
+        this.cache?.set(key, {
+            resolvedPath,
+            value: null,
+            kind: 'resolve',
+        });
+        return resolvedPath;
     }
     public resolveImported(
         imported: Imported,

diff --git a/packages/core/test/stylable-resolver.spec.ts b/packages/core/test/stylable-resolver.spec.ts
@@ -1,36 +1,21 @@
 import { expect } from 'chai';
 import type * as postcss from 'postcss';
 import { testStylableCore, generateStylableResult } from '@stylable/core-test-kit';
-import { Stylable, MinimalFS, StylableMeta, createDefaultResolver } from '@stylable/core';
-import { StylableResolver, cachedProcessFile } from '@stylable/core/dist/index-internal';
+import { Stylable, MinimalFS } from '@stylable/core';
 
 function createResolveExtendsResults(
     fileSystem: MinimalFS,
     fileToProcess: string,
     classNameToLookup: string,
     isElement = false
 ) {
-    const moduleResolver = createDefaultResolver(fileSystem, {});
     const stylable = new Stylable({
         fileSystem,
         projectRoot: '/',
     });
 
-    const processFile = cachedProcessFile<StylableMeta>(
-        (fullPath, content) => {
-            return stylable.analyze(fullPath, content);
-        },
-        (filePath: string) => fileSystem.readFileSync(filePath, 'utf8')
-    );
-
-    const resolver = new StylableResolver(
-        processFile,
-        (module: string) => module && '',
-        (context = '/', request: string) => moduleResolver(context, request)
-    );
-
-    return resolver.resolveExtends(
-        processFile.process(fileToProcess),
+    return stylable.resolver.resolveExtends(
+        stylable.analyze(fileToProcess),
         classNameToLookup,
         isElement
     );
@@ -404,4 +389,25 @@ describe('stylable-resolver', () => {
         expect(meta.diagnostics.reports).to.eql([]);
         expect(meta.transformDiagnostics!.reports).to.eql([]);
     });
+
+    it('should not hit the underling resolver more then once', () => {
+        let resolverHits = 0;
+        const { stylable } = testStylableCore(
+            {},
+            {
+                stylableConfig: {
+                    resolverCache: new Map(),
+                    resolveModule: () => {
+                        resolverHits++;
+                        return '';
+                    },
+                },
+            }
+        );
+
+        stylable.resolver.resolvePath('/', './entry.st.css');
+        stylable.resolver.resolvePath('/', './entry.st.css');
+
+        expect(resolverHits).to.equal(1);
+    });
 });
diff --git a/packages/webpack-extensions/test/e2e/manifest.css-vars.spec.ts b/packages/webpack-extensions/test/e2e/manifest.css-vars.spec.ts
@@ -10,7 +10,7 @@ const projectDir = dirname(
     require.resolve(`@stylable/webpack-extensions/test/e2e/projects/${project}/webpack.config`)
 );
 
-describe(`${project} - manifest`, () => {
+describe(`${project} - manifest (vars)`, () => {
     const projectRunner = StylableProjectRunner.mochaSetup(
         {
             projectDir,

diff --git a/packages/webpack-extensions/test/e2e/manifest.spec.ts b/packages/webpack-extensions/test/e2e/manifest.spec.ts
@@ -10,7 +10,7 @@ const projectDir = dirname(
     require.resolve(`@stylable/webpack-extensions/test/e2e/projects/${project}/webpack.config`)
 );
 
-describe(`${project} - manifest`, () => {
+describe(`${project} - manifest (e2e)`, () => {
     const projectRunner = StylableProjectRunner.mochaSetup(
         {
             projectDir,