Skip to content

Commit

Permalink
fix(astro): Fix import path when using external init files with defau…
Browse files Browse the repository at this point in the history
…lt path (#10214)

Fix a bug in the Astro SDK where specifying an
external `sentry.(client|server).config.js` in the default location file
would cause a build error.

Previously, we checked for a supported file extension and instead of
adding an import statement for the path to the file, we'd only add the
file extension. So
```js
import 'mjs';
// instead of 
import 'path/to/sentry.client.config.mjs'
```

Fix the filename lookup function and adds a test to cover
regressions.
  • Loading branch information
Lms24 committed Jan 17, 2024
1 parent fb7e516 commit 70d1cbb
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ export default defineConfig({
sourceMapsUploadOptions: {
enabled: false,
},
clientInitPath: 'sentry.client.mjs',
// purposefully setting the default name for client
// and a custom name for server to test both cases
serverInitPath: 'sentry.server.mjs',
}),
],
Expand Down
7 changes: 3 additions & 4 deletions packages/astro/src/integration/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,10 +120,9 @@ const possibleFileExtensions = ['ts', 'js', 'tsx', 'jsx', 'mjs', 'cjs', 'mts'];

function findDefaultSdkInitFile(type: 'server' | 'client'): string | undefined {
const cwd = process.cwd();
return possibleFileExtensions.find(extension => {
const filename = path.resolve(path.join(cwd, `sentry.${type}.config.${extension}`));
return fs.existsSync(filename);
});
return possibleFileExtensions
.map(e => path.resolve(path.join(cwd, `sentry.${type}.config.${e}`)))
.find(filename => fs.existsSync(filename));
}

function getSourcemapsAssetsGlob(config: AstroConfig): string {
Expand Down
41 changes: 41 additions & 0 deletions packages/astro/test/integration/index.files.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import { vi } from 'vitest';

import { sentryAstro } from '../../src/integration';

vi.mock('fs', async () => {
const actual = await vi.importActual('fs');
return {
// @ts-expect-error - just mocking around
...actual,
existsSync: vi.fn(p => p.endsWith('js')),
};
});

const updateConfig = vi.fn();
const injectScript = vi.fn();
const config = {
root: new URL('file://path/to/project'),
outDir: new URL('file://path/to/project/out'),
};

describe('sentryAstro integration', () => {
afterEach(() => {
vi.clearAllMocks();
});

it('injects client and server init scripts from default paths if they exist', () => {
const integration = sentryAstro({});

expect(integration.hooks['astro:config:setup']).toBeDefined();

// @ts-expect-error - the hook exists and we only need to pass what we actually use
integration.hooks['astro:config:setup']({ updateConfig, injectScript, config });

expect(injectScript).toHaveBeenCalledTimes(2);
expect(injectScript).toHaveBeenCalledWith('page', expect.stringMatching(/^import ".*\/sentry.client.config.js"/));
expect(injectScript).toHaveBeenCalledWith(
'page-ssr',
expect.stringMatching(/^import ".*\/sentry.server.config.js"/),
);
});
});
7 changes: 5 additions & 2 deletions packages/astro/test/integration/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -239,8 +239,11 @@ describe('sentryAstro integration', () => {
await integration.hooks['astro:config:setup']({ updateConfig, injectScript, config });

expect(injectScript).toHaveBeenCalledTimes(2);
expect(injectScript).toHaveBeenCalledWith('page', expect.stringContaining('my-client-init-path.js'));
expect(injectScript).toHaveBeenCalledWith('page-ssr', expect.stringContaining('my-server-init-path.js'));
expect(injectScript).toHaveBeenCalledWith('page', expect.stringMatching(/^import ".*\/my-client-init-path.js"/));
expect(injectScript).toHaveBeenCalledWith(
'page-ssr',
expect.stringMatching(/^import ".*\/my-server-init-path.js"/),
);
});

it.each(['server', 'hybrid'])(
Expand Down

0 comments on commit 70d1cbb

Please sign in to comment.