Skip to content
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

Script detection doesn't work in current versions of SvelteKit #24

Open
Rich-Harris opened this issue Mar 3, 2023 · 6 comments
Open

Comments

@Rich-Harris
Copy link

See sveltejs/kit#9277 — since sveltejs/kit#8901, this logic...

function externalizeScript(html, assets) {
return html.replace(
/<script type="module" data-hydrate="([\s\S]+)">([\s\S]+)<\/script>/,
(match, hydrationTarget, content) => {
const hash = Buffer.from(hash_script(content), 'base64').toString('hex');
const externalized_script_path = join(assets, `${hash}.js`);
writeFileSync(externalized_script_path, content);
return `<script type="module" data-hydrate="${hydrationTarget}" src="${hash}.js"></script>`;
}
);
}

...no longer works, because the <script> no longer has type="module" (as this would defeat streaming). The 'proper' solution might well be to finally implement sveltejs/kit#3555

@bogacg
Copy link

bogacg commented Apr 10, 2023

@Rich-Harris I did manage to fix that part of the problem in my local repo, however now I get this error when the extension is executed:

Uncaught TypeError: Cannot read properties of null (reading 'parentElement')

from generated (and externalised script)

	 __sveltekit_1yt2rwz = {
		env: {},
		base: new URL(".", location).pathname.slice(0, -1),
		element: document.currentScript.parentElement
	};

	const data = [null,null];

	Promise.all([
		import("./app/immutable/entry/start.e005700e.js"),
		import("./app/immutable/entry/app.ae8f820f.js")
	]).then(([kit, app]) => {
		kit.start(app, __sveltekit_1yt2rwz.element, {
			node_ids: [0, 2],
			data,
			form: null,
			error: null
		});
	});

I do get document object, but currentScript is null, as well as __sveltekit_1yt2rwz.

Do you have any useful pointers to fix that?

Reading that JavaScript I think we need to do more than externalized module generation from inline script.

@bogacg
Copy link

bogacg commented Apr 10, 2023

Digging further into this, I believe fix needs to come from SvelteKit side. As mentioned by @Rich-Harris sveltejs/kit#3555 needs to be implemented because modules have no currentScript reach yet; therefore turning code in the example above into an external module is useless.

There is an ongoing discussion at WHATWG started years ago. 🤦🏻‍♂️

Side note: create-react-app had the same problem apparently and then they fixed it.

Edit

If anyone like me excited about both Svelte and extension development, until this gets fixed we may try Astro and plain Svelte route. Did not fully tested myself yet , will update here.

Update

Astro generates inline script in the output as well.

@antony
Copy link
Owner

antony commented May 25, 2023

Apparently have not got notifications for this repo turned on! Will need to investigate the ins and outs of this issue to fully understand it.

@maietta
Copy link

maietta commented Jun 24, 2023

bump!

@riverfr0zen
Copy link

riverfr0zen commented Sep 11, 2023

If you need a workaround and don't want to downgrade Sveltekit to 1.7.2, the removeInlineScript.cjs script shown in this tutorial worked for me.

I just copied the script to my project and appended it to the npm build command as suggested in the tutorial. Seems to do the trick for me, whereas before I was getting CSP errors about inline scripts.

Edit: I had to add tiny-glob as a dep since the script uses it:
npm add --save-dev tiny-glob

@LineageFalcon
Copy link

It still would be nice to have an solid build in solution from sveltekits side. Really disappointing to see that issue going on for over 3 years and not much happening in regard to that. And it only is one single inline placement.

In regard to that normal Svelte doesn't seem to have that problem. (Maybe cuz it does not utilize the routing feature.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants