Skip to content

feat(sveltekit): Read adapter output directory from svelte.config.js #7863

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions packages/sveltekit/src/vite/sentryVitePlugins.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ const DEFAULT_PLUGIN_OPTIONS: SentrySvelteKitPluginOptions = {
* Sentry adds a few additional properties to your Vite config.
* Make sure, it is registered before the SvelteKit plugin.
*/
export function sentrySvelteKit(options: SentrySvelteKitPluginOptions = {}): Plugin[] {
export async function sentrySvelteKit(options: SentrySvelteKitPluginOptions = {}): Promise<Plugin[]> {
Copy link
Member Author

@Lms24 Lms24 Apr 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is technically breaking as it changes the return type of the function but the good news is that noone has to await this function as the vite config is perfectly fine with taking a Promise<Plugin[]>. I should have done this already but for some reason decided to stay sync in #7811. I vote we go forward with making it async, not just because we need it for this change but also because it gives us more freedom in the future.

Also, we're still in alpha, so yolo...

const mergedOptions = {
...DEFAULT_PLUGIN_OPTIONS,
...options,
Expand All @@ -50,7 +50,7 @@ export function sentrySvelteKit(options: SentrySvelteKitPluginOptions = {}): Plu
...mergedOptions.sourceMapsUploadOptions,
debug: mergedOptions.debug, // override the plugin's debug flag with the one from the top-level options
};
sentryPlugins.push(makeCustomSentryVitePlugin(pluginOptions));
sentryPlugins.push(await makeCustomSentryVitePlugin(pluginOptions));
}

return sentryPlugins;
Expand Down
38 changes: 22 additions & 16 deletions packages/sveltekit/src/vite/sourceMaps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,7 @@ import * as path from 'path';
import * as sorcery from 'sorcery';
import type { Plugin } from 'vite';

const DEFAULT_PLUGIN_OPTIONS: SentryVitePluginOptions = {
// TODO: Read these values from the node adapter somehow as the out dir can be changed in the adapter options
include: [
{ paths: ['build/client'] },
{ paths: ['build/server/chunks'] },
{ paths: ['build/server'], ignore: ['chunks/**'] },
],
};
import { getAdapterOutputDir, loadSvelteConfig } from './svelteConfig';

// sorcery has no types, so these are some basic type definitions:
type Chain = {
Expand Down Expand Up @@ -45,17 +38,30 @@ type SentryVitePluginOptionsOptionalInclude = Omit<SentryVitePluginOptions, 'inc
*
* @returns the custom Sentry Vite plugin
*/
export function makeCustomSentryVitePlugin(options?: SentryVitePluginOptionsOptionalInclude): Plugin {
export async function makeCustomSentryVitePlugin(options?: SentryVitePluginOptionsOptionalInclude): Promise<Plugin> {
const svelteConfig = await loadSvelteConfig();

const outputDir = await getAdapterOutputDir(svelteConfig);

const defaultPluginOptions: SentryVitePluginOptions = {
include: [
{ paths: [`${outputDir}/client`] },
{ paths: [`${outputDir}/server/chunks`] },
{ paths: [`${outputDir}/server`], ignore: ['chunks/**'] },
],
};

const mergedOptions = {
...DEFAULT_PLUGIN_OPTIONS,
...defaultPluginOptions,
...options,
};

const sentryPlugin: Plugin = sentryVitePlugin(mergedOptions);

const { debug } = mergedOptions;
const { buildStart, resolveId, transform, renderChunk } = sentryPlugin;

let upload = true;
let isSSRBuild = true;

const customPlugin: Plugin = {
name: 'sentry-vite-plugin-custom',
Expand Down Expand Up @@ -88,19 +94,19 @@ export function makeCustomSentryVitePlugin(options?: SentryVitePluginOptionsOpti
// `config.build.ssr` is `true` for that first build and `false` in the other ones.
// Hence we can use it as a switch to upload source maps only once in main build.
if (!config.build.ssr) {
upload = false;
isSSRBuild = false;
}
},

// We need to start uploading source maps later than in the original plugin
// because SvelteKit is still doing some stuff at closeBundle.
// because SvelteKit is invoking the adapter at closeBundle.
// This means that we need to wait until the adapter is done before we start uploading.
closeBundle: async () => {
if (!upload) {
if (!isSSRBuild) {
return;
}

// TODO: Read the out dir from the node adapter somehow as it can be changed in the adapter options
const outDir = path.resolve(process.cwd(), 'build');
const outDir = path.resolve(process.cwd(), outputDir);

const jsFiles = getFiles(outDir).filter(file => file.endsWith('.js'));
// eslint-disable-next-line no-console
Expand Down
94 changes: 94 additions & 0 deletions packages/sveltekit/src/vite/svelteConfig.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
/* eslint-disable @sentry-internal/sdk/no-optional-chaining */

import type { Builder, Config } from '@sveltejs/kit';
import * as fs from 'fs';
import * as path from 'path';
import * as url from 'url';

/**
* Imports the svelte.config.js file and returns the config object.
* The sveltekit plugins import the config in the same way.
* See: https://github.com/sveltejs/kit/blob/master/packages/kit/src/core/config/index.js#L63
*/
export async function loadSvelteConfig(): Promise<Config> {
// This can only be .js (see https://github.com/sveltejs/kit/pull/4031#issuecomment-1049475388)
const SVELTE_CONFIG_FILE = 'svelte.config.js';

const configFile = path.join(process.cwd(), SVELTE_CONFIG_FILE);

try {
if (!fs.existsSync(configFile)) {
return {};
}
// @ts-ignore - we explicitly want to import the svelte config here.
const svelteConfigModule = await import(`${url.pathToFileURL(configFile).href}`);

// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
return (svelteConfigModule?.default as Config) || {};
} catch (e) {
// eslint-disable-next-line no-console
console.warn("[Source Maps Plugin] Couldn't load svelte.config.js:");
// eslint-disable-next-line no-console
console.log(e);

return {};
}
}

/**
* Attempts to read a custom output directory that can be specidied in the options
* of a SvelteKit adapter. If no custom output directory is specified, the default
* directory is returned.
*
* To get the directory, we have to apply a hack and call the adapter's adapt method
* with a custom adapter `Builder` that only calls the `writeClient` method.
* This method is the first method that is called with the output directory.
* Once we obtained the output directory, we throw an error to exit the adapter.
*
* see: https://github.com/sveltejs/kit/blob/master/packages/adapter-node/index.js#L17
*
*/
export async function getAdapterOutputDir(svelteConfig: Config): Promise<string> {
// 'build' is the default output dir for the node adapter
let outputDir = 'build';

if (!svelteConfig.kit?.adapter) {
return outputDir;
}

const adapter = svelteConfig.kit.adapter;

const adapterBuilder: Builder = {
writeClient(dest: string) {
outputDir = dest.replace(/\/client.*/, '');
throw new Error('We got what we came for, throwing to exit the adapter');
},
// @ts-ignore - No need to implement the other methods
log: {
// eslint-disable-next-line @typescript-eslint/no-empty-function -- this should be a noop
minor() {},
},
getBuildDirectory: () => '',
// eslint-disable-next-line @typescript-eslint/no-empty-function -- this should be a noop
rimraf: () => {},
// eslint-disable-next-line @typescript-eslint/no-empty-function -- this should be a noop
mkdirp: () => {},

config: {
kit: {
// @ts-ignore - the builder expects a validated config but for our purpose it's fine to just pass this partial config
paths: {
base: svelteConfig.kit?.paths?.base || '',
},
},
},
};

try {
await adapter.adapt(adapterBuilder);
} catch (_) {
// We expect the adapter to throw in writeClient!
}

return outputDir;
}
16 changes: 8 additions & 8 deletions packages/sveltekit/test/vite/sentrySvelteKitPlugins.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,26 +4,26 @@ import { sentrySvelteKit } from '../../src/vite/sentryVitePlugins';
import * as sourceMaps from '../../src/vite/sourceMaps';

describe('sentryVite()', () => {
it('returns an array of Vite plugins', () => {
const plugins = sentrySvelteKit();
it('returns an array of Vite plugins', async () => {
const plugins = await sentrySvelteKit();
expect(plugins).toBeInstanceOf(Array);
expect(plugins).toHaveLength(1);
});

it('returns the custom sentry source maps plugin by default', () => {
const plugins = sentrySvelteKit();
it('returns the custom sentry source maps plugin by default', async () => {
const plugins = await sentrySvelteKit();
const plugin = plugins[0];
expect(plugin.name).toEqual('sentry-vite-plugin-custom');
});

it("doesn't return the custom sentry source maps plugin if autoUploadSourcemaps is `false`", () => {
const plugins = sentrySvelteKit({ autoUploadSourceMaps: false });
it("doesn't return the custom sentry source maps plugin if autoUploadSourcemaps is `false`", async () => {
const plugins = await sentrySvelteKit({ autoUploadSourceMaps: false });
expect(plugins).toHaveLength(0);
});

it('passes user-specified vite pugin options to the custom sentry source maps plugin', () => {
it('passes user-specified vite pugin options to the custom sentry source maps plugin', async () => {
const makePluginSpy = vi.spyOn(sourceMaps, 'makeCustomSentryVitePlugin');
const plugins = sentrySvelteKit({
const plugins = await sentrySvelteKit({
debug: true,
sourceMapsUploadOptions: {
include: ['foo.js'],
Expand Down
38 changes: 30 additions & 8 deletions packages/sveltekit/test/vite/sourceMaps.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ beforeEach(() => {
});

describe('makeCustomSentryVitePlugin()', () => {
it('returns the custom sentry source maps plugin', () => {
const plugin = makeCustomSentryVitePlugin();
it('returns the custom sentry source maps plugin', async () => {
const plugin = await makeCustomSentryVitePlugin();
expect(plugin.name).toEqual('sentry-vite-plugin-custom');
expect(plugin.apply).toEqual('build');
expect(plugin.enforce).toEqual('post');
Expand All @@ -41,8 +41,8 @@ describe('makeCustomSentryVitePlugin()', () => {
});

describe('Custom sentry vite plugin', () => {
it('enables source map generation', () => {
const plugin = makeCustomSentryVitePlugin();
it('enables source map generation', async () => {
const plugin = await makeCustomSentryVitePlugin();
// @ts-ignore this function exists!
const sentrifiedConfig = plugin.config({ build: { foo: {} }, test: {} });
expect(sentrifiedConfig).toEqual({
Expand All @@ -54,17 +54,17 @@ describe('makeCustomSentryVitePlugin()', () => {
});
});

it('uploads source maps during the SSR build', () => {
const plugin = makeCustomSentryVitePlugin();
it('uploads source maps during the SSR build', async () => {
const plugin = await makeCustomSentryVitePlugin();
// @ts-ignore this function exists!
plugin.configResolved({ build: { ssr: true } });
// @ts-ignore this function exists!
plugin.closeBundle();
expect(mockedSentryVitePlugin.writeBundle).toHaveBeenCalledTimes(1);
});

it("doesn't upload source maps during the non-SSR builds", () => {
const plugin = makeCustomSentryVitePlugin();
it("doesn't upload source maps during the non-SSR builds", async () => {
const plugin = await makeCustomSentryVitePlugin();

// @ts-ignore this function exists!
plugin.configResolved({ build: { ssr: false } });
Expand All @@ -73,4 +73,26 @@ describe('makeCustomSentryVitePlugin()', () => {
expect(mockedSentryVitePlugin.writeBundle).not.toHaveBeenCalled();
});
});

it('catches errors while uploading source maps', async () => {
mockedSentryVitePlugin.writeBundle.mockImplementationOnce(() => {
throw new Error('test error');
});

const consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
const consoleLogSpy = vi.spyOn(console, 'log').mockImplementation(() => {});

const plugin = await makeCustomSentryVitePlugin();

// @ts-ignore this function exists!
expect(plugin.closeBundle).not.toThrow();

// @ts-ignore this function exists!
plugin.configResolved({ build: { ssr: true } });
// @ts-ignore this function exists!
plugin.closeBundle();

expect(consoleWarnSpy).toHaveBeenCalledWith(expect.stringContaining('Failed to upload source maps'));
expect(consoleLogSpy).toHaveBeenCalled();
});
});
69 changes: 69 additions & 0 deletions packages/sveltekit/test/vite/svelteConfig.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
import { vi } from 'vitest';

import { getAdapterOutputDir, loadSvelteConfig } from '../../src/vite/svelteConfig';

let existsFile;

describe('loadSvelteConfig', () => {
vi.mock('fs', () => {
return {
existsSync: () => existsFile,
};
});

vi.mock(`${process.cwd()}/svelte.config.js`, () => {
return {
default: {
kit: {
adapter: {},
},
},
};
});

// url apparently doesn't exist in the test environment, therefore we mock it:
vi.mock('url', () => {
return {
pathToFileURL: path => {
return {
href: path,
};
},
};
});

beforeEach(() => {
existsFile = true;
vi.clearAllMocks();
});

it('returns the svelte config', async () => {
const config = await loadSvelteConfig();
expect(config).toStrictEqual({
kit: {
adapter: {},
},
});
});

it('returns an empty object if svelte.config.js does not exist', async () => {
existsFile = false;

const config = await loadSvelteConfig();
expect(config).toStrictEqual({});
});
});

describe('getAdapterOutputDir', () => {
const mockedAdapter = {
name: 'mocked-adapter',
adapt(builder) {
builder.writeClient('customBuildDir');
},
};

it('returns the output directory of the adapter', async () => {
const outputDir = await getAdapterOutputDir({ kit: { adapter: mockedAdapter } });
expect(outputDir).toEqual('customBuildDir');
});
});