From b1ad0d0b3db09bd42e898ea6c151f0aa1ab37efb Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Tue, 23 May 2023 17:47:04 +0200 Subject: [PATCH 1/3] feat(sveltekit): Auto-detect SvelteKit adapters --- packages/sveltekit/src/vite/detectAdapter.ts | 58 +++++++++++++++++ .../sveltekit/src/vite/sentryVitePlugins.ts | 24 +++++++ .../sveltekit/test/vite/detectAdapter.test.ts | 63 +++++++++++++++++++ .../test/vite/sentrySvelteKitPlugins.test.ts | 7 +++ 4 files changed, 152 insertions(+) create mode 100644 packages/sveltekit/src/vite/detectAdapter.ts create mode 100644 packages/sveltekit/test/vite/detectAdapter.test.ts diff --git a/packages/sveltekit/src/vite/detectAdapter.ts b/packages/sveltekit/src/vite/detectAdapter.ts new file mode 100644 index 000000000000..d6ed870402ad --- /dev/null +++ b/packages/sveltekit/src/vite/detectAdapter.ts @@ -0,0 +1,58 @@ +import * as fs from 'fs'; +import * as path from 'path'; + +/** + * Supported @sveltejs/adapters-[adapter] SvelteKit adapters + */ +export type SupportedSvelteKitAdapters = 'node' | 'auto' | 'vercel' | 'other'; + +type PackageJson = { + dependencies?: Record>; + devDependencies?: Record>; +} & Record; + +/** + * Tries to detect the used adapter for SvelteKit by looking at the dependencies. + * returns the name of the adapter or 'other' if no supported adapter was found. + */ +export async function detectAdapter(): Promise { + const pkgJson = await loadPackageJson(); + + const allDependencies = [...Object.keys(pkgJson.dependencies || {}), ...Object.keys(pkgJson.devDependencies || {})]; + + if (allDependencies.find(dep => dep === '@sveltejs/adapter-vercel')) { + return 'vercel'; + } + if (allDependencies.find(dep => dep === '@sveltejs/adapter-node')) { + return 'node'; + } + if (allDependencies.find(dep => dep === '@sveltejs/adapter-auto')) { + return 'auto'; + } + + return 'other'; +} + +/** + * Imports the pacakge.json file and returns the parsed JSON object. + */ +async function loadPackageJson(): Promise { + const pkgFile = path.join(process.cwd(), 'package.json'); + + try { + if (!fs.existsSync(pkgFile)) { + throw `${pkgFile} does not exist}`; + } + + const pkgJsonContent = (await fs.promises.readFile(pkgFile, 'utf-8')).toString(); + + return JSON.parse(pkgJsonContent); + } catch (e) { + // eslint-disable-next-line no-console + console.warn("[Sentry SvelteKit Plugin] Couldn't load package.json:"); + // eslint-disable-next-line no-console + console.log(e); + + return {}; + } +} diff --git a/packages/sveltekit/src/vite/sentryVitePlugins.ts b/packages/sveltekit/src/vite/sentryVitePlugins.ts index 1f983d7a9653..1140e15a7ef7 100644 --- a/packages/sveltekit/src/vite/sentryVitePlugins.ts +++ b/packages/sveltekit/src/vite/sentryVitePlugins.ts @@ -3,6 +3,8 @@ import type { Plugin } from 'vite'; import type { AutoInstrumentSelection } from './autoInstrument'; import { makeAutoInstrumentationPlugin } from './autoInstrument'; +import type { SupportedSvelteKitAdapters } from './detectAdapter'; +import { detectAdapter } from './detectAdapter'; import { makeCustomSentryVitePlugin } from './sourceMaps'; type SourceMapsUploadOptions = { @@ -39,6 +41,24 @@ export type SentrySvelteKitPluginOptions = { * @default false. */ debug?: boolean; + + /** + * Specify which SvelteKit adapter you're using. + * By default, the SDK will attempt auto-detect the used adapter at build time and apply the + * correct config for source maps upload or auto-instrumentation. + * + * Currently, the SDK supports the following adapters: + * - node (@sveltejs/adapter-node) + * - auto (@sveltejs/adapter-auto) only Vercel + * - vercel (@sveltejs/adapter-auto) only Serverless functions, no edge runtime + * + * Set this option, if the SDK detects the wrong adapter or you want to use an adapter + * that is not in this list. If you specify 'other', you'll most likely need to configure + * source maps upload yourself. + * + * @default {} the SDK attempts to auto-detect the used adapter at build time + */ + adapter?: SupportedSvelteKitAdapters; } & SourceMapsUploadOptions & AutoInstrumentOptions; @@ -79,6 +99,10 @@ export async function sentrySvelteKit(options: SentrySvelteKitPluginOptions = {} } if (mergedOptions.autoUploadSourceMaps && process.env.NODE_ENV !== 'development') { + // TODO: pass to makeCosutmSentryVitePlugin + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const adapter = mergedOptions.adapter || (await detectAdapter()); + const pluginOptions = { ...mergedOptions.sourceMapsUploadOptions, debug: mergedOptions.debug, // override the plugin's debug flag with the one from the top-level options diff --git a/packages/sveltekit/test/vite/detectAdapter.test.ts b/packages/sveltekit/test/vite/detectAdapter.test.ts new file mode 100644 index 000000000000..66189d69dd34 --- /dev/null +++ b/packages/sveltekit/test/vite/detectAdapter.test.ts @@ -0,0 +1,63 @@ +import { vi } from 'vitest'; + +import { detectAdapter } from '../../src/vite/detectAdapter'; + +let existsFile = true; +const pkgJson = { + dependencies: {}, +}; +describe('detectAdapter', () => { + beforeEach(() => { + existsFile = true; + vi.clearAllMocks(); + pkgJson.dependencies = {}; + }); + + vi.mock('fs', () => { + return { + existsSync: () => existsFile, + promises: { + readFile: () => { + return Promise.resolve(JSON.stringify(pkgJson)); + }, + }, + }; + }); + + it.each(['auto', 'vercel', 'node'])('returns the adapter name (adapter %s)', async adapter => { + pkgJson.dependencies[`@sveltejs/adapter-${adapter}`] = '1.0.0'; + const detectedAdapter = await detectAdapter(); + expect(detectedAdapter).toEqual(adapter); + }); + + it('returns "other" if no supported adapter was found', async () => { + pkgJson.dependencies['@sveltejs/adapter-netlify'] = '1.0.0'; + const detectedAdapter = await detectAdapter(); + expect(detectedAdapter).toEqual('other'); + }); + + it('returns "other" if package.json isnt available and emits a warning log', async () => { + const consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const consoleLogSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + + existsFile = false; + const detectedAdapter = await detectAdapter(); + expect(detectedAdapter).toEqual('other'); + + expect(consoleWarnSpy).toHaveBeenCalledTimes(1); + expect(consoleLogSpy).toHaveBeenCalledTimes(1); + }); + + it('prefers all other adapters over adapter auto', async () => { + pkgJson.dependencies['@sveltejs/adapter-auto'] = '1.0.0'; + pkgJson.dependencies['@sveltejs/adapter-vercel'] = '1.0.0'; + pkgJson.dependencies['@sveltejs/adapter-node'] = '1.0.0'; + + const detectedAdapter = await detectAdapter(); + expect(detectedAdapter).toEqual('vercel'); + + delete pkgJson.dependencies['@sveltejs/adapter-vercel']; + const detectedAdapter2 = await detectAdapter(); + expect(detectedAdapter2).toEqual('node'); + }); +}); diff --git a/packages/sveltekit/test/vite/sentrySvelteKitPlugins.test.ts b/packages/sveltekit/test/vite/sentrySvelteKitPlugins.test.ts index 963844bdef70..1259085b2dcb 100644 --- a/packages/sveltekit/test/vite/sentrySvelteKitPlugins.test.ts +++ b/packages/sveltekit/test/vite/sentrySvelteKitPlugins.test.ts @@ -17,6 +17,13 @@ vi.mock('fs', async () => { }; }); +vi.spyOn(console, 'log').mockImplementation(() => { + /* noop */ +}); +vi.spyOn(console, 'warn').mockImplementation(() => { + /* noop */ +}); + describe('sentryVite()', () => { it('returns an array of Vite plugins', async () => { const plugins = await sentrySvelteKit(); From 05efc66efea8a87d1c91c013e6cb3fe205abb12c Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Tue, 23 May 2023 18:20:27 +0200 Subject: [PATCH 2/3] add console output --- packages/sveltekit/src/vite/detectAdapter.ts | 37 +++++++++++-------- .../sveltekit/src/vite/sentryVitePlugins.ts | 5 +-- .../sveltekit/test/vite/detectAdapter.test.ts | 29 +++++++++++---- packages/types/src/package.ts | 2 + 4 files changed, 46 insertions(+), 27 deletions(-) diff --git a/packages/sveltekit/src/vite/detectAdapter.ts b/packages/sveltekit/src/vite/detectAdapter.ts index d6ed870402ad..1e21aa43b307 100644 --- a/packages/sveltekit/src/vite/detectAdapter.ts +++ b/packages/sveltekit/src/vite/detectAdapter.ts @@ -1,3 +1,4 @@ +import type { Package } from '@sentry/types'; import * as fs from 'fs'; import * as path from 'path'; @@ -6,42 +7,48 @@ import * as path from 'path'; */ export type SupportedSvelteKitAdapters = 'node' | 'auto' | 'vercel' | 'other'; -type PackageJson = { - dependencies?: Record>; - devDependencies?: Record>; -} & Record; - /** * Tries to detect the used adapter for SvelteKit by looking at the dependencies. * returns the name of the adapter or 'other' if no supported adapter was found. */ -export async function detectAdapter(): Promise { +export async function detectAdapter(debug?: boolean): Promise { const pkgJson = await loadPackageJson(); const allDependencies = [...Object.keys(pkgJson.dependencies || {}), ...Object.keys(pkgJson.devDependencies || {})]; + let adapter: SupportedSvelteKitAdapters = 'other'; if (allDependencies.find(dep => dep === '@sveltejs/adapter-vercel')) { - return 'vercel'; - } - if (allDependencies.find(dep => dep === '@sveltejs/adapter-node')) { - return 'node'; + adapter = 'vercel'; + } else if (allDependencies.find(dep => dep === '@sveltejs/adapter-node')) { + adapter = 'node'; + } else if (allDependencies.find(dep => dep === '@sveltejs/adapter-auto')) { + adapter = 'auto'; } - if (allDependencies.find(dep => dep === '@sveltejs/adapter-auto')) { - return 'auto'; + + if (debug) { + if (adapter === 'other') { + // eslint-disable-next-line no-console + console.warn( + "[Sentry SvelteKit Plugin] Couldn't detect SvelteKit adapter. Please set the 'adapter' option manually.", + ); + } else { + // eslint-disable-next-line no-console + console.log(`[Sentry SvelteKit Plugin] Detected SvelteKit ${adapter} adapter`); + } } - return 'other'; + return adapter; } /** * Imports the pacakge.json file and returns the parsed JSON object. */ -async function loadPackageJson(): Promise { +async function loadPackageJson(): Promise> { const pkgFile = path.join(process.cwd(), 'package.json'); try { if (!fs.existsSync(pkgFile)) { - throw `${pkgFile} does not exist}`; + throw `File ${pkgFile} doesn't exist}`; } const pkgJsonContent = (await fs.promises.readFile(pkgFile, 'utf-8')).toString(); diff --git a/packages/sveltekit/src/vite/sentryVitePlugins.ts b/packages/sveltekit/src/vite/sentryVitePlugins.ts index 1140e15a7ef7..e78a25adea10 100644 --- a/packages/sveltekit/src/vite/sentryVitePlugins.ts +++ b/packages/sveltekit/src/vite/sentryVitePlugins.ts @@ -79,6 +79,7 @@ export async function sentrySvelteKit(options: SentrySvelteKitPluginOptions = {} const mergedOptions = { ...DEFAULT_PLUGIN_OPTIONS, ...options, + adapter: options.adapter || (await detectAdapter(options.debug || false)), }; const sentryPlugins: Plugin[] = []; @@ -99,10 +100,6 @@ export async function sentrySvelteKit(options: SentrySvelteKitPluginOptions = {} } if (mergedOptions.autoUploadSourceMaps && process.env.NODE_ENV !== 'development') { - // TODO: pass to makeCosutmSentryVitePlugin - // eslint-disable-next-line @typescript-eslint/no-unused-vars - const adapter = mergedOptions.adapter || (await detectAdapter()); - const pluginOptions = { ...mergedOptions.sourceMapsUploadOptions, debug: mergedOptions.debug, // override the plugin's debug flag with the one from the top-level options diff --git a/packages/sveltekit/test/vite/detectAdapter.test.ts b/packages/sveltekit/test/vite/detectAdapter.test.ts index 66189d69dd34..33fb28b6945a 100644 --- a/packages/sveltekit/test/vite/detectAdapter.test.ts +++ b/packages/sveltekit/test/vite/detectAdapter.test.ts @@ -24,11 +24,19 @@ describe('detectAdapter', () => { }; }); - it.each(['auto', 'vercel', 'node'])('returns the adapter name (adapter %s)', async adapter => { - pkgJson.dependencies[`@sveltejs/adapter-${adapter}`] = '1.0.0'; - const detectedAdapter = await detectAdapter(); - expect(detectedAdapter).toEqual(adapter); - }); + const consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const consoleLogSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + + it.each(['auto', 'vercel', 'node'])( + 'returns the adapter name (adapter %s) and logs it to the console', + async adapter => { + pkgJson.dependencies[`@sveltejs/adapter-${adapter}`] = '1.0.0'; + const detectedAdapter = await detectAdapter(true); + expect(detectedAdapter).toEqual(adapter); + expect(consoleLogSpy).toHaveBeenCalledTimes(1); + expect(consoleLogSpy).toHaveBeenCalledWith(expect.stringContaining(`Detected SvelteKit ${adapter} adapter`)); + }, + ); it('returns "other" if no supported adapter was found', async () => { pkgJson.dependencies['@sveltejs/adapter-netlify'] = '1.0.0'; @@ -36,10 +44,15 @@ describe('detectAdapter', () => { expect(detectedAdapter).toEqual('other'); }); - it('returns "other" if package.json isnt available and emits a warning log', async () => { - const consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); - const consoleLogSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + it('logs a warning if in debug mode and no supported adapter was found', async () => { + pkgJson.dependencies['@sveltejs/adapter-netlify'] = '1.0.0'; + const detectedAdapter = await detectAdapter(true); + expect(detectedAdapter).toEqual('other'); + expect(consoleWarnSpy).toHaveBeenCalledTimes(1); + expect(consoleWarnSpy).toHaveBeenCalledWith(expect.stringContaining("Couldn't detect SvelteKit adapter")); + }); + it('returns "other" if package.json isnt available and emits a warning log', async () => { existsFile = false; const detectedAdapter = await detectAdapter(); expect(detectedAdapter).toEqual('other'); diff --git a/packages/types/src/package.ts b/packages/types/src/package.ts index c44d66e62950..15a61f3668d3 100644 --- a/packages/types/src/package.ts +++ b/packages/types/src/package.ts @@ -2,4 +2,6 @@ export interface Package { name: string; version: string; + dependencies?: Record; + devDependencies?: Record; } From 8bd96f774f80dc341670a1371bde99ded8f8dbc6 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 24 May 2023 15:51:58 +0200 Subject: [PATCH 3/3] code review suggestions --- packages/sveltekit/src/vite/detectAdapter.ts | 19 ++++++++----------- .../sveltekit/test/vite/detectAdapter.test.ts | 5 ++++- .../test/vite/sentrySvelteKitPlugins.test.ts | 2 +- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/packages/sveltekit/src/vite/detectAdapter.ts b/packages/sveltekit/src/vite/detectAdapter.ts index 1e21aa43b307..95e26dfc9f6d 100644 --- a/packages/sveltekit/src/vite/detectAdapter.ts +++ b/packages/sveltekit/src/vite/detectAdapter.ts @@ -14,14 +14,14 @@ export type SupportedSvelteKitAdapters = 'node' | 'auto' | 'vercel' | 'other'; export async function detectAdapter(debug?: boolean): Promise { const pkgJson = await loadPackageJson(); - const allDependencies = [...Object.keys(pkgJson.dependencies || {}), ...Object.keys(pkgJson.devDependencies || {})]; + const allDependencies = pkgJson ? { ...pkgJson.dependencies, ...pkgJson.devDependencies } : {}; let adapter: SupportedSvelteKitAdapters = 'other'; - if (allDependencies.find(dep => dep === '@sveltejs/adapter-vercel')) { + if (allDependencies['@sveltejs/adapter-vercel']) { adapter = 'vercel'; - } else if (allDependencies.find(dep => dep === '@sveltejs/adapter-node')) { + } else if (allDependencies['@sveltejs/adapter-node']) { adapter = 'node'; - } else if (allDependencies.find(dep => dep === '@sveltejs/adapter-auto')) { + } else if (allDependencies['@sveltejs/adapter-auto']) { adapter = 'auto'; } @@ -43,12 +43,12 @@ export async function detectAdapter(debug?: boolean): Promise> { +async function loadPackageJson(): Promise { const pkgFile = path.join(process.cwd(), 'package.json'); try { if (!fs.existsSync(pkgFile)) { - throw `File ${pkgFile} doesn't exist}`; + throw new Error(`File ${pkgFile} doesn't exist}`); } const pkgJsonContent = (await fs.promises.readFile(pkgFile, 'utf-8')).toString(); @@ -56,10 +56,7 @@ async function loadPackageJson(): Promise> { return JSON.parse(pkgJsonContent); } catch (e) { // eslint-disable-next-line no-console - console.warn("[Sentry SvelteKit Plugin] Couldn't load package.json:"); - // eslint-disable-next-line no-console - console.log(e); - - return {}; + console.warn("[Sentry SvelteKit Plugin] Couldn't load package.json:", e); + return undefined; } } diff --git a/packages/sveltekit/test/vite/detectAdapter.test.ts b/packages/sveltekit/test/vite/detectAdapter.test.ts index 33fb28b6945a..7f4e05a1a44b 100644 --- a/packages/sveltekit/test/vite/detectAdapter.test.ts +++ b/packages/sveltekit/test/vite/detectAdapter.test.ts @@ -58,7 +58,10 @@ describe('detectAdapter', () => { expect(detectedAdapter).toEqual('other'); expect(consoleWarnSpy).toHaveBeenCalledTimes(1); - expect(consoleLogSpy).toHaveBeenCalledTimes(1); + expect(consoleWarnSpy).toHaveBeenCalledWith( + expect.stringContaining("Couldn't load package.json"), + expect.any(Error), + ); }); it('prefers all other adapters over adapter auto', async () => { diff --git a/packages/sveltekit/test/vite/sentrySvelteKitPlugins.test.ts b/packages/sveltekit/test/vite/sentrySvelteKitPlugins.test.ts index 1259085b2dcb..026d347d777d 100644 --- a/packages/sveltekit/test/vite/sentrySvelteKitPlugins.test.ts +++ b/packages/sveltekit/test/vite/sentrySvelteKitPlugins.test.ts @@ -24,7 +24,7 @@ vi.spyOn(console, 'warn').mockImplementation(() => { /* noop */ }); -describe('sentryVite()', () => { +describe('sentrySvelteKit()', () => { it('returns an array of Vite plugins', async () => { const plugins = await sentrySvelteKit();