Fix weird bug in AssetSizeAppBlock{CSS,JavaScript} (#314)

We were getting "Can't find file size" for assets/app.js errors being
logged. That was because the function wasn't being called with an
absolute path but with a relative path.

For fuck's sake. I refactored our root finding algo for a long time
thinking it was the cause. It wasn't.

It was this fucking dumb implementation bug.

Fixes Shopify/cli#3497

.changeset/silly-snakes-relax.md

@@ -0,0 +1,5 @@
+---
+'@shopify/theme-check-common': patch
+---
+
+Fix AssetAppBlock{CSS,JavaScript} bug when cwd != project root

packages/prettier-plugin-liquid/package.json

@@ -40,7 +40,7 @@
     "prettier": "scripts/prettier",
     "prettier2": "cross-env PRETTIER_MAJOR=2 scripts/prettier",
     "prettier3": "cross-env PRETTIER_MAJOR=3 scripts/prettier",
-    "test": "vitest -c \"./vitest.config.ts\"",
+    "test": "vitest -c \"./vitest.config.mjs\"",
     "test:3": "cross-env PRETTIER_MAJOR=3 yarn test",
     "test:idempotence": "cross-env TEST_IDEMPOTENCE=true vitest run 'src/test/'",
     "test:idempotence:3": "cross-env PRETTIER_MAJOR=3 yarn test:idempotence",

packages/theme-check-common/src/checks/asset-size-app-block-css/index.ts

@@ -5,7 +5,7 @@ import {
   Severity,
   SourceCodeType,
 } from '../../types';
-import { assertFileExists, assertFileSize } from '../../utils/file-utils';
+import { doesFileExist, doesFileExceedThreshold } from '../../utils/file-utils';
 
 const schema = {
   thresholdInBytes: SchemaProp.number(100000),
@@ -46,13 +46,13 @@ export const AssetSizeAppBlockCSS: LiquidCheckDefinition<typeof schema> = {
           return;
         }
 
-        const absolutePath = `assets/${filePath}`;
+        const relativePath = `assets/${filePath}`;
         const thresholdInBytes = context.settings.thresholdInBytes;
 
         const startIndex = node.body.position.start + node.body.value.indexOf(filePath);
         const endIndex = startIndex + filePath.length;
 
-        const fileExists = await assertFileExists(context, absolutePath);
+        const fileExists = await doesFileExist(context, relativePath);
 
         if (!fileExists) {
           context.report({
@@ -63,8 +63,11 @@ export const AssetSizeAppBlockCSS: LiquidCheckDefinition<typeof schema> = {
           return;
         }
 
-        const fileSize = await context.fileSize!(absolutePath);
-        const fileExceedsThreshold = await assertFileSize(thresholdInBytes, fileSize);
+        const [fileExceedsThreshold, fileSize] = await doesFileExceedThreshold(
+          context,
+          relativePath,
+          thresholdInBytes,
+        );
 
         if (fileExceedsThreshold) {
           context.report({

packages/theme-check-common/src/checks/asset-size-app-block-javascript/index.ts

@@ -5,7 +5,7 @@ import {
   Severity,
   SourceCodeType,
 } from '../../types';
-import { assertFileExists, assertFileSize } from '../../utils/file-utils';
+import { doesFileExist, doesFileExceedThreshold } from '../../utils/file-utils';
 
 const schema = {
   thresholdInBytes: SchemaProp.number(10000),
@@ -46,13 +46,13 @@ export const AssetSizeAppBlockJavaScript: LiquidCheckDefinition<typeof schema> =
           return;
         }
 
-        const absolutePath = `assets/${filePath}`;
+        const relativePath = `assets/${filePath}`;
         const thresholdInBytes = context.settings.thresholdInBytes;
 
         const startIndex = node.body.position.start + node.body.value.indexOf(filePath);
         const endIndex = startIndex + filePath.length;
 
-        const fileExists = await assertFileExists(context, absolutePath);
+        const fileExists = await doesFileExist(context, relativePath);
 
         if (!fileExists) {
           context.report({
@@ -63,8 +63,11 @@ export const AssetSizeAppBlockJavaScript: LiquidCheckDefinition<typeof schema> =
           return;
         }
 
-        const fileSize = await context.fileSize!(absolutePath);
-        const fileExceedsThreshold = await assertFileSize(thresholdInBytes, fileSize);
+        const [fileExceedsThreshold, fileSize] = await doesFileExceedThreshold(
+          context,
+          relativePath,
+          thresholdInBytes,
+        );
 
         if (fileExceedsThreshold) {
           context.report({

packages/theme-check-common/src/checks/asset-size-css/index.ts

@@ -74,7 +74,9 @@ export const AssetSizeCSS: LiquidCheckDefinition<typeof schema> = {
     }
 
     async function checkThemeAssetSize(srcValue: string, position: { start: number; end: number }) {
-      if (await hasLocalAssetSizeExceededThreshold(context, srcValue, thresholdInBytes)) {
+      if (
+        await hasLocalAssetSizeExceededThreshold(context, `assets/${srcValue}`, thresholdInBytes)
+      ) {
         context.report({
           message: `The CSS file size exceeds the threshold of ${thresholdInBytes} bytes`,
           startIndex: position.start,

packages/theme-check-common/src/checks/asset-size-javascript/index.ts

@@ -72,7 +72,9 @@ export const AssetSizeJavaScript: LiquidCheckDefinition<typeof schema> = {
     }
 
     async function checkThemeAssetSize(srcValue: string, position: { start: number; end: number }) {
-      if (await hasLocalAssetSizeExceededThreshold(context, srcValue, thresholdInBytes)) {
+      if (
+        await hasLocalAssetSizeExceededThreshold(context, `assets/${srcValue}`, thresholdInBytes)
+      ) {
         context.report({
           message: `JavaScript on every page load exceeds compressed size threshold (${thresholdInBytes} Bytes), consider using the import on interaction pattern.`,
           startIndex: position.start,

packages/theme-check-common/src/checks/missing-asset/index.ts

@@ -1,5 +1,5 @@
 import { LiquidCheckDefinition, Severity, SourceCodeType } from '../../types';
-import { assertFileExists } from '../../utils/file-utils';
+import { doesFileExist } from '../../utils/file-utils';
 import { isLiquidString } from '../utils';
 
 export const MissingAsset: LiquidCheckDefinition = {
@@ -30,18 +30,18 @@ export const MissingAsset: LiquidCheckDefinition = {
         let originalAssetPath = `assets/${expression.value}`;
         let assetPath = originalAssetPath;
 
-        let fileExists = await assertFileExists(context, assetPath);
+        let fileExists = await doesFileExist(context, assetPath);
         if (fileExists) return;
 
         if (assetPath.endsWith('.scss.css')) {
           assetPath = assetPath.replace('.scss.css', '.scss.liquid');
-          fileExists = await assertFileExists(context, assetPath);
+          fileExists = await doesFileExist(context, assetPath);
           if (fileExists) return;
         }
 
         if (assetPath.endsWith('.js') || assetPath.endsWith('.css')) {
           assetPath += '.liquid';
-          fileExists = await assertFileExists(context, assetPath);
+          fileExists = await doesFileExist(context, assetPath);
           if (fileExists) return;
         }
 

packages/theme-check-common/src/checks/missing-template/index.ts

@@ -13,7 +13,7 @@ import {
   Severity,
   SourceCodeType,
 } from '../../types';
-import { assertFileExists } from '../../utils/file-utils';
+import { doesFileExist } from '../../utils/file-utils';
 
 const schema = {
   ignoreMissing: SchemaProp.array(SchemaProp.string(), []),
@@ -46,7 +46,7 @@ export const MissingTemplate: LiquidCheckDefinition<typeof schema> = {
       relativePath: RelativePath,
       { position }: { position: Position },
     ) {
-      const fileExists = await assertFileExists(context, relativePath);
+      const fileExists = await doesFileExist(context, relativePath);
       if (fileExists || isIgnored(relativePath)) return;
 
       context.report({

packages/theme-check-common/src/test/test-helper.ts

@@ -56,7 +56,7 @@ export async function check(
     checks: checks,
     root: '/',
   };
-  const defaultTranslationsFileAbsolutePath = 'locales/en.default.json';
+  const defaultTranslationsFileRelativePath = 'locales/en.default.json';
   const defaultMockDependencies = {
     async fileSize(absolutePath: string) {
       const relativePath = absolutePath.replace(/^\//, '');
@@ -68,14 +68,14 @@ export async function check(
     },
     async getDefaultTranslations() {
       try {
-        return JSON.parse(themeDesc[defaultTranslationsFileAbsolutePath] || '{}');
+        return JSON.parse(themeDesc[defaultTranslationsFileRelativePath] || '{}');
       } catch (e) {
         if (e instanceof SyntaxError) return {};
         throw e;
       }
     },
     async getDefaultLocale() {
-      return defaultTranslationsFileAbsolutePath.match(/locales\/(.*)\.default\.json$/)?.[1]!;
+      return defaultTranslationsFileRelativePath.match(/locales\/(.*)\.default\.json$/)?.[1]!;
     },
     themeDocset: {
       async filters() {

packages/theme-check-common/src/utils/file-utils.ts

@@ -1,20 +1,23 @@
 import { Context, SourceCodeType, Schema, RelativePath } from '../types';
 import { fetch } from 'cross-fetch';
 
-export async function assertFileExists<T extends SourceCodeType, S extends Schema>(
+export async function doesFileExist<T extends SourceCodeType, S extends Schema>(
   context: Context<T, S>,
-  filePath: RelativePath,
+  relativePath: RelativePath,
 ): Promise<boolean> {
-  const absolutePath = context.absolutePath(filePath);
+  const absolutePath = context.absolutePath(relativePath);
   return await context.fileExists(absolutePath);
 }
 
-export async function assertFileSize<T extends SourceCodeType, S extends Schema>(
+export async function doesFileExceedThreshold<T extends SourceCodeType, S extends Schema>(
+  context: Context<T, S>,
+  relativePath: RelativePath,
   thresholdInBytes: number,
-  fileSize: number,
-): Promise<boolean> {
-  if (fileSize <= thresholdInBytes) return false;
-  return true;
+): Promise<[exceeds: boolean, fileSize: number]> {
+  const absolutePath = context.absolutePath(relativePath);
+  if (!context.fileSize) return [false, 0];
+  const fileSize = await context.fileSize(absolutePath);
+  return [fileSize > thresholdInBytes, fileSize];
 }
 
 export async function getFileSize(url: string): Promise<number> {
@@ -41,13 +44,15 @@ export async function hasRemoteAssetSizeExceededThreshold(url: string, threshold
 export async function hasLocalAssetSizeExceededThreshold<
   T extends SourceCodeType,
   S extends Schema,
->(context: Context<T, S>, path: string, thresholdInBytes: number) {
-  const absolutePath = `assets/${path}`;
-  const fileExists = await assertFileExists(context, absolutePath);
-
-  if (!fileExists) return;
-  const fileSize = await context.fileSize!(absolutePath);
-  const fileExceedsThreshold = await assertFileSize(thresholdInBytes, fileSize);
+>(context: Context<T, S>, relativePath: RelativePath, thresholdInBytes: number) {
+  const fileExists = await doesFileExist(context, relativePath);
+  if (!fileExists) return false;
+
+  const [fileExceedsThreshold, _fileSize] = await doesFileExceedThreshold(
+    context,
+    relativePath,
+    thresholdInBytes,
+  );
 
   return fileExceedsThreshold;
 }