ref(sveltekit): Improve auto-instrumentation applicability check (#8083)

Walk AST to check for `load` function instead of string/regex based approach

Author: Lms24

packages/sveltekit/package.json

@@ -28,6 +28,7 @@
     "@sentry/types": "7.53.0",
     "@sentry/utils": "7.53.0",
     "@sentry/vite-plugin": "^0.6.0",
+    "magicast": "0.2.6",
     "sorcery": "0.11.0"
   },
   "devDependencies": {

packages/sveltekit/src/vite/autoInstrument.ts

@@ -1,5 +1,7 @@
 /* eslint-disable @sentry-internal/sdk/no-optional-chaining */
+import type { ExportNamedDeclaration } from '@babel/types';
 import * as fs from 'fs';
+import { parseModule } from 'magicast';
 import * as path from 'path';
 import type { Plugin } from 'vite';
 
@@ -89,24 +91,50 @@ export function makeAutoInstrumentationPlugin(options: AutoInstrumentPluginOptio
  */
 export async function canWrapLoad(id: string, debug: boolean): Promise<boolean> {
   const code = (await fs.promises.readFile(id, 'utf8')).toString();
+  const mod = parseModule(code);
 
-  const codeWithoutComments = code.replace(/(\/\/.*| ?\/\*[^]*?\*\/)(,?)$/gm, '');
-
-  const hasSentryContent = codeWithoutComments.includes('@sentry/sveltekit');
-  if (hasSentryContent) {
+  const program = mod.$ast.type === 'Program' && mod.$ast;
+  if (!program) {
     // eslint-disable-next-line no-console
-    debug && console.log(`Skipping wrapping ${id} because it already contains Sentry code`);
+    debug && console.log(`Skipping wrapping ${id} because it doesn't contain valid JavaScript or TypeScript`);
+    return false;
   }
 
-  const hasLoadDeclaration = /((const|let|var|function)\s+load\s*(=|\(|:))|as\s+load\s*(,|})/gm.test(
-    codeWithoutComments,
-  );
+  const hasLoadDeclaration = program.body
+    .filter((statement): statement is ExportNamedDeclaration => statement.type === 'ExportNamedDeclaration')
+    .find(exportDecl => {
+      // find `export const load = ...`
+      if (exportDecl.declaration && exportDecl.declaration.type === 'VariableDeclaration') {
+        const variableDeclarations = exportDecl.declaration.declarations;
+        return variableDeclarations.find(decl => decl.id.type === 'Identifier' && decl.id.name === 'load');
+      }
+
+      // find `export function load = ...`
+      if (exportDecl.declaration && exportDecl.declaration.type === 'FunctionDeclaration') {
+        const functionId = exportDecl.declaration.id;
+        return functionId?.name === 'load';
+      }
+
+      // find `export { load, somethingElse as load, somethingElse as "load" }`
+      if (exportDecl.specifiers) {
+        return exportDecl.specifiers.find(specifier => {
+          return (
+            (specifier.exported.type === 'Identifier' && specifier.exported.name === 'load') ||
+            (specifier.exported.type === 'StringLiteral' && specifier.exported.value === 'load')
+          );
+        });
+      }
+
+      return false;
+    });
+
   if (!hasLoadDeclaration) {
     // eslint-disable-next-line no-console
     debug && console.log(`Skipping wrapping ${id} because it doesn't declare a \`load\` function`);
+    return false;
   }
 
-  return !hasSentryContent && hasLoadDeclaration;
+  return true;
 }
 
 /**

packages/sveltekit/test/vite/autoInstrument.test.ts

@@ -121,6 +121,7 @@ describe('canWrapLoad', () => {
     ['export variable declaration - function pointer', 'export const load=   loadPageData'],
     ['export variable declaration - factory function call', 'export const load    =loadPageData()'],
     ['export variable declaration - inline function', 'export const load = () => { return { props: { msg: "hi" } } }'],
+    ['export variable declaration - inline function let', 'export let load = () => {}'],
     [
       'export variable declaration - inline async function',
       'export const load = async () => { return { props: { msg: "hi" } } }',
@@ -139,14 +140,14 @@ describe('canWrapLoad', () => {
       'variable declaration (let)',
       `import {something} from 'somewhere';
       let load = async () => {};
-      export prerender = true;
+      export const prerender = true;
       export { load}`,
     ],
     [
       'variable declaration (var)',
       `import {something} from 'somewhere';
       var    load=async () => {};
-      export prerender = true;
+      export const prerender = true;
       export { load}`,
     ],
 
@@ -176,13 +177,18 @@ describe('canWrapLoad', () => {
        async function somethingElse(){};
        export { somethingElse as  load, foo }`,
     ],
-
+    [
+      'function declaration with different string literal name',
+      `import { foo } from 'somewhere';
+       async function somethingElse(){};
+       export { somethingElse as "load", foo }`,
+    ],
     [
       'export variable declaration - inline function with assigned type',
       `import type { LayoutLoad } from './$types';
        export const load :  LayoutLoad = async () => { return { props: { msg: "hi" } } }`,
     ],
-  ])('returns `true` if a load declaration  (%s) exists and no Sentry code was found', async (_, code) => {
+  ])('returns `true` if a load declaration  (%s) exists', async (_, code) => {
     fileContent = code;
     expect(await canWrapLoad('+page.ts', false)).toEqual(true);
   });
@@ -203,14 +209,4 @@ describe('canWrapLoad', () => {
     fileContent = code;
     expect(await canWrapLoad('+page.ts', false)).toEqual(true);
   });
-
-  it('returns `false` if Sentry code was found', async () => {
-    fileContent = 'import * as Sentry from "@sentry/sveltekit";';
-    expect(await canWrapLoad('+page.ts', false)).toEqual(false);
-  });
-
-  it('returns `false` if Sentry code was found', async () => {
-    fileContent = 'import * as Sentry from "@sentry/sveltekit";';
-    expect(await canWrapLoad('+page.ts', false)).toEqual(false);
-  });
 });

yarn.lock

@@ -970,6 +970,11 @@
   resolved "https://registry.yarnpkg.com/@babel/helper-string-parser/-/helper-string-parser-7.19.4.tgz#38d3acb654b4701a9b77fb0615a96f775c3a9e63"
   integrity sha512-nHtDoQcuqFmwYNYPz3Rah5ph2p8PFeFCsZk9A/48dPc/rGocJ5J3hAAZ7pb76VWX3fZKu+uEr/FhH5jLx7umrw==
 
+"@babel/helper-string-parser@^7.21.5":
+  version "7.21.5"
+  resolved "https://registry.yarnpkg.com/@babel/helper-string-parser/-/helper-string-parser-7.21.5.tgz#2b3eea65443c6bdc31c22d037c65f6d323b6b2bd"
+  integrity sha512-5pTUx3hAJaZIdW99sJ6ZUUgWq/Y+Hja7TowEnLNMm1VivRgZQL3vpBY3qUACVsvw+yQU6+YgfBVmcbLaZtrA1w==
+
 "@babel/helper-validator-identifier@^7.18.6", "@babel/helper-validator-identifier@^7.19.1":
   version "7.19.1"
   resolved "https://registry.yarnpkg.com/@babel/helper-validator-identifier/-/helper-validator-identifier-7.19.1.tgz#7eea834cf32901ffdc1a7ee555e2f9c27e249ca2"
@@ -1027,6 +1032,11 @@
   resolved "https://registry.yarnpkg.com/@babel/parser/-/parser-7.20.15.tgz#eec9f36d8eaf0948bb88c87a46784b5ee9fd0c89"
   integrity sha512-DI4a1oZuf8wC+oAJA9RW6ga3Zbe8RZFt7kD9i4qAspz3I/yHet1VvC3DiSy/fsUvv5pvJuNPh0LPOdCcqinDPg==
 
+"@babel/parser@^7.21.8":
+  version "7.21.8"
+  resolved "https://registry.yarnpkg.com/@babel/parser/-/parser-7.21.8.tgz#642af7d0333eab9c0ad70b14ac5e76dbde7bfdf8"
+  integrity sha512-6zavDGdzG3gUqAdWvlLFfk+36RilI+Pwyuuh7HItyeScCWP3k6i8vKclAQ0bM/0y/Kz/xiwvxhMv9MgTJP5gmA==
+
 "@babel/plugin-bugfix-safari-id-destructuring-collision-in-function-expression@^7.18.6":
   version "7.18.6"
   resolved "https://registry.yarnpkg.com/@babel/plugin-bugfix-safari-id-destructuring-collision-in-function-expression/-/plugin-bugfix-safari-id-destructuring-collision-in-function-expression-7.18.6.tgz#da5b8f9a580acdfbe53494dba45ea389fb09a4d2"
@@ -2218,6 +2228,15 @@
     "@babel/helper-validator-identifier" "^7.19.1"
     to-fast-properties "^2.0.0"
 
+"@babel/types@^7.21.5":
+  version "7.21.5"
+  resolved "https://registry.yarnpkg.com/@babel/types/-/types-7.21.5.tgz#18dfbd47c39d3904d5db3d3dc2cc80bedb60e5b6"
+  integrity sha512-m4AfNvVF2mVC/F7fDEdH2El3HzUg9It/XsCxZiOTTA3m3qYfcSVSbTfM6Q9xG+hYDniZssYhlXKKUMD5m8tF4Q==
+  dependencies:
+    "@babel/helper-string-parser" "^7.21.5"
+    "@babel/helper-validator-identifier" "^7.19.1"
+    to-fast-properties "^2.0.0"
+
 "@bcoe/v8-coverage@^0.2.3":
   version "0.2.3"
   resolved "https://registry.yarnpkg.com/@bcoe/v8-coverage/-/v8-coverage-0.2.3.tgz#75a2e8b51cb758a7553d6804a5932d7aace75c39"
@@ -6565,6 +6584,13 @@ ast-types@0.14.2:
   dependencies:
     tslib "^2.0.1"
 
+ast-types@0.15.2:
+  version "0.15.2"
+  resolved "https://registry.yarnpkg.com/ast-types/-/ast-types-0.15.2.tgz#39ae4809393c4b16df751ee563411423e85fb49d"
+  integrity sha512-c27loCv9QkZinsa5ProX751khO9DJl/AcB5c2KNtA6NRvHKS0PgLfcftz72KVq504vB0Gku5s2kUZzDBvQWvHg==
+  dependencies:
+    tslib "^2.0.1"
+
 astral-regex@^2.0.0:
   version "2.0.0"
   resolved "https://registry.yarnpkg.com/astral-regex/-/astral-regex-2.0.0.tgz#483143c567aeed4785759c0865786dc77d7d2e31"
@@ -17972,6 +17998,15 @@ magic-string@^0.30.0:
   dependencies:
     "@jridgewell/sourcemap-codec" "^1.4.13"
 
+magicast@0.2.6:
+  version "0.2.6"
+  resolved "https://registry.yarnpkg.com/magicast/-/magicast-0.2.6.tgz#08c9f1900177ca1896e9c07981912171d4ed8ec1"
+  integrity sha512-6bX0nVjGrA41o+qHSv9Duiv3VuF7jUyjT7dIb3E61YW/5mucvCBMgyZssUznRc+xlUMPYyXZZluZjE1k5z+2yQ==
+  dependencies:
+    "@babel/parser" "^7.21.8"
+    "@babel/types" "^7.21.5"
+    recast "^0.22.0"
+
 make-dir@3.1.0, make-dir@^3.0.0, make-dir@^3.0.2, make-dir@^3.1.0, make-dir@~3.1.0:
   version "3.1.0"
   resolved "https://registry.yarnpkg.com/make-dir/-/make-dir-3.1.0.tgz#415e967046b3a7f1d185277d84aa58203726a13f"
@@ -22902,6 +22937,17 @@ recast@^0.20.5:
     source-map "~0.6.1"
     tslib "^2.0.1"
 
+recast@^0.22.0:
+  version "0.22.0"
+  resolved "https://registry.yarnpkg.com/recast/-/recast-0.22.0.tgz#1dd3bf1b86e5eb810b044221a1a734234ed3e9c0"
+  integrity sha512-5AAx+mujtXijsEavc5lWXBPQqrM4+Dl5qNH96N2aNeuJFUzpiiToKPsxQD/zAIJHspz7zz0maX0PCtCTFVlixQ==
+  dependencies:
+    assert "^2.0.0"
+    ast-types "0.15.2"
+    esprima "~4.0.0"
+    source-map "~0.6.1"
+    tslib "^2.0.1"
+
 rechoir@^0.6.2:
   version "0.6.2"
   resolved "https://registry.yarnpkg.com/rechoir/-/rechoir-0.6.2.tgz#85204b54dba82d5742e28c96756ef43af50e3384"