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

Svelte island does not expect class prop by default, causing warning #5665

Closed
1 task
Shmuppel opened this issue Dec 22, 2022 · 3 comments · Fixed by #7291
Closed
1 task

Svelte island does not expect class prop by default, causing warning #5665

Shmuppel opened this issue Dec 22, 2022 · 3 comments · Fixed by #7291
Labels
- P2: nice to have Not breaking anything but nice to have (priority) pkg: svelte Related to Svelte (scope)

Comments

@Shmuppel
Copy link

What version of astro are you using?

1.7.2

Are you using an SSR adapter? If so, which one?

None

What package manager are you using?

npm

What operating system are you using?

Mac

Describe the Bug

A lot of work has already gone into passing Astro classes to Svelte islands (#5397, #5112), currently it seems the default behaviour is to pass the scoped classname as a prop (class="astro-VFKJLXQS"). However, Svelte components by default don't expect this prop unless explicitly used in the Svelte component. This causes a console warning: <Example> was created with unknown prop 'class'.

I'm hesitant to bring it up because it seems like more of a Svelte issue than an Astro one, but it does cause unexpected warnings which can be considered bad DX.

Thanks guys!

Link to Minimal Reproducible Example

https://codesandbox.io/p/sandbox/sweet-james-kxt2e3

Participation

  • I am willing to submit a pull request for this issue.
@Princesseuh Princesseuh added the - P2: nice to have Not breaking anything but nice to have (priority) label Jan 12, 2023
@narration-sd
Copy link

narration-sd commented Jan 25, 2023

Just spent a time on this myself, without discovering an external workaround, but indeed tis is something apparently internal to Svelte...not to be fixed, as it's supposed to be a dev 'warning'' to 'help'.

Someone ought to speak to them about the psychology of this, but the good thing is that it and others do go away for production. You can prove it to yourself by doing a build and then preview with your Astro project.

Still agree well with @Princesseuh 's categorization...

reference: sveltejs/svelte#4652 (comment)

@SrJSDev
Copy link

SrJSDev commented Feb 2, 2023

Just came here from a Google search, after a similar incident while using Astro + Svelte:

index.svelte? [sm]:20 <App> was created with unknown prop 'class'

Not crazy bad or anything, but annoying, as I cannot even export let class = "" in my Svelte component - it's invalid JS syntax.

Is there a reason Astro must pass in a class prop?

@HiDeoo
Copy link
Member

HiDeoo commented Feb 3, 2023

A dirty workaround could be the @astrojs/svelte renderer clientEntrypoint containing a console filter like the ones already existing in Astro, e.g. this one or the one in the preact integration, filtering out warnings specifically targeting the class prop.

It looks like SvelteKit is already using a similar approach for filtering out the data & form prop warnings:

if (DEV) {
	// Nasty hack to silence harmless warnings the user can do nothing about
	const console_warn = console.warn;
	console.warn = function warn(...args) {
		if (
			args.length === 1 &&
			/<(Layout|Page|Error)(_[\w$]+)?> was created (with unknown|without expected) prop '(data|form)'/.test(
				args[0]
			)
		) {
			return;
		}
		console_warn(...args);
	};
}

kitschpatrol added a commit to kitschpatrol/astro that referenced this issue Jun 4, 2023
Astro's hydration code passes a `class` prop to Svelte components, inducing Svelte to log a warning about an unknown prop. Preempting this by exporting a `class` prop from the Svelte component isn't a viable workaround since `class` is a reserved identifier in JS.

This PR implements the console-filtering workaround suggested by @HiDeoo in withastro#5665, borrowing the `useConsoleFilter` approach from the [preact integration](https://github.com/withastro/astro/blob/a1c0cbe604c9f91cdc421b5606aab574999eba01/packages/integrations/preact/src/server.ts#L72-L94).

It would probably be better to generalize console filtering so it could be shared across multiple integrations.

Ideally there would be a way to handle this in Svelte, but as was pointed out in the issue thread even they resort to [similar cringe-inducing hackery](https://github.com/sveltejs/kit/blob/master/packages/kit/src/runtime/client/client.js#L1974-L1996) in sveltekit.
@natemoo-re natemoo-re added the pkg: svelte Related to Svelte (scope) label Aug 9, 2023
@natemoo-re natemoo-re linked a pull request Aug 10, 2023 that will close this issue
natemoo-re added a commit that referenced this issue Aug 15, 2023
* Filter out Svelte's unexpected class prop console warnings

Astro's hydration code passes a `class` prop to Svelte components, inducing Svelte to log a warning about an unknown prop. Preempting this by exporting a `class` prop from the Svelte component isn't a viable workaround since `class` is a reserved identifier in JS.

This PR implements the console-filtering workaround suggested by @HiDeoo in #5665, borrowing the `useConsoleFilter` approach from the [preact integration](https://github.com/withastro/astro/blob/a1c0cbe604c9f91cdc421b5606aab574999eba01/packages/integrations/preact/src/server.ts#L72-L94).

It would probably be better to generalize console filtering so it could be shared across multiple integrations.

Ideally there would be a way to handle this in Svelte, but as was pointed out in the issue thread even they resort to [similar cringe-inducing hackery](https://github.com/sveltejs/kit/blob/master/packages/kit/src/runtime/client/client.js#L1974-L1996) in sveltekit.

* Only filter Svelte console warnings in dev builds

* Add changeset

* Fix lint error.

---------

Co-authored-by: bluwy <bjornlu.dev@gmail.com>
Co-authored-by: Nate Moore <natemoo-re@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P2: nice to have Not breaking anything but nice to have (priority) pkg: svelte Related to Svelte (scope)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants