From e11dbfe6466eb3dba99d6fb24bb4d13591a22d13 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 14 Oct 2022 09:45:09 +0200 Subject: [PATCH 1/4] fix(svelte): Track components without is important! Without any content, + // the `script` hook wouldn't be executed for the added script tag. + const s = new MagicString(content); + s.prepend('\n'); + return { code: s.toString(), map: s.generateMap().toString() }; + } + + return { code: content }; + }, + // This script hook is called whenever a Svelte component's \n

I'm just a plain component

\n", + ); }); - expectComponentCodeToBeModified([cmp11, cmp2], { trackInit: true, trackUpdates: true }); - expect(cmp12.newCode).toEqual(cmp12.originalCode); + it("doesn't add a \n

I'm a component with a script

\n", + filename: 'lib/Cmp2.svelte', + name: 'Cmp2', + }; + + const res: any = + preProc.markup && + preProc.markup({ + content: component.originalCode, + filename: component.filename, + }); + + expect(res.code).toEqual( + "\n

I'm a component with a script

\n", + ); + }); }); - it('doesnt inject the function call to a module context script block', () => { - const preProc = componentTrackingPreprocessor(); - const component = { - originalCode: 'console.log(cmp2)', - filename: 'lib/Cmp2.svelte', - name: 'Cmp2', - }; + // These are more "higher level" tests in which we use the actual preprocessing command of the Svelte compiler + // This lets us test all preprocessor hooks we use in the correct order + describe('all hooks combined, using the svelte compiler', () => { + it('handles components without script tags correctly', async () => { + const component = { + originalCode: "

I'm just a plain component

\n", + filename: 'lib/Cmp1.svelte', + }; - const res: any = - preProc.script && - preProc.script({ - content: component.originalCode, + const processedCode = await svelteCompiler.preprocess(component.originalCode, [componentTrackingPreprocessor()], { filename: component.filename, - attributes: { context: 'module' }, - markup: '', }); - const processedComponent = { ...component, newCode: res.code, map: res.map }; + expect(processedCode.code).toEqual( + '\n' + + "

I'm just a plain component

\n" + + '', + ); + }); + + it('handles components with script tags correctly', async () => { + const component = { + originalCode: + "\n

I'm a component with a script

\n", + filename: 'lib/Cmp2.svelte', + }; + + const processedCode = await svelteCompiler.preprocess(component.originalCode, [componentTrackingPreprocessor()], { + filename: component.filename, + }); - expect(processedComponent.newCode).toEqual(processedComponent.originalCode); + expect(processedCode.code).toEqual( + '\n" + + "

I'm a component with a script

\n" + + '', + ); + }); + + it("falls back to 'unknown' if filename isn't passed to preprocessor", async () => { + const component = { + originalCode: "", + filename: undefined, + }; + + const processedCode = await svelteCompiler.preprocess(component.originalCode, [componentTrackingPreprocessor()], { + filename: component.filename, + }); + + expect(processedCode.code).toEqual( + '", + ); + }); }); }); From 731cdc02901def058cdee19625ce91d975d81a38 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 14 Oct 2022 11:23:54 +0200 Subject: [PATCH 2/4] switch from regex check to svelteCompiler.parse --- packages/svelte/rollup.npm.config.js | 2 +- packages/svelte/src/preprocessors.ts | 17 +++++------------ 2 files changed, 6 insertions(+), 13 deletions(-) diff --git a/packages/svelte/rollup.npm.config.js b/packages/svelte/rollup.npm.config.js index ae3c7a9a5b8a..c2ab32e60374 100644 --- a/packages/svelte/rollup.npm.config.js +++ b/packages/svelte/rollup.npm.config.js @@ -3,6 +3,6 @@ import { makeBaseNPMConfig, makeNPMConfigVariants } from '../../rollup/index.js' export default makeNPMConfigVariants( makeBaseNPMConfig({ // Prevent 'svelte/internal' stuff from being included in the built JS - packageSpecificConfig: { external: ['svelte/internal'] }, + packageSpecificConfig: { external: ['svelte/internal', 'svelte/compiler'] }, }), ); diff --git a/packages/svelte/src/preprocessors.ts b/packages/svelte/src/preprocessors.ts index 2ee399f864f9..5ab80cf4c120 100644 --- a/packages/svelte/src/preprocessors.ts +++ b/packages/svelte/src/preprocessors.ts @@ -1,4 +1,5 @@ import MagicString from 'magic-string'; +import * as svelteCompiler from 'svelte/compiler'; import { PreprocessorGroup } from 'svelte/types/compiler/preprocess'; import { ComponentTrackingInitOptions, SentryPreprocessorGroup, TrackComponentOptions } from './types'; @@ -123,16 +124,8 @@ function getBaseName(filename: string): string { } function hasScriptTag(content: string): boolean { - // This regex is taken from the Svelte compiler code. - // They use this regex to find matching script tags that are passed to the `script` preprocessor hook: - // https://github.com/sveltejs/svelte/blob/bb83eddfc623437528f24e9fe210885b446e72fa/src/compiler/preprocess/index.ts#L144 - // However, we remove the first part of the regex to not match HTML comments - const scriptTagRegex = /([^]*?)<\/script>|\/>)/gi; - - // Regex testing is not a super safe way of checking for the presence of a