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.

Sign up for GitHub

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

Jump to bottom

Update Svelte integration v6 docs #10002

Merged
merged 2 commits into from
Nov 15, 2024
Merged

Update Svelte integration v6 docs #10002

merged 2 commits into from
Nov 15, 2024

Conversation

bluwy
Copy link
Member

@bluwy bluwy commented Nov 13, 2024

Description (required)

Updates docs around new behaviour for @astrojs/svelte v6

Related issues & labels (optional)

  • Closes #
  • Suggested label:

For @astrojs/svelte version: 6.0. See astro PR withastro/astro#12364

@bluwy bluwy added improve or update documentation Enhance / update existing documentation (e.g. add example, improve description, update for changes) merge-on-release Don't merge this before the feature is released! (MQ=approved but WAIT for feature release!) labels Nov 13, 2024
@bluwy bluwy mentioned this pull request Nov 13, 2024
Merged
@netlify Netlify
Copy link

netlify bot commented Nov 13, 2024 •
edited
Loading

✅ Deploy Preview for astro-docs-2 ready!

Name Link
🔨 Latest commit 8c1011f
🔍 Latest deploy log https://app.netlify.com/sites/astro-docs-2/deploys/673732a30d444d000820ad47
😎 Deploy Preview https://deploy-preview-10002--astro-docs-2.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@astrobot-houston
Copy link
Contributor

astrobot-houston commented Nov 13, 2024 •
edited
Loading

Lunaria Status Overview

🌕 This pull request will trigger status changes.

Learn more

By default, every PR changing files present in the Lunaria configuration's files property will be considered and trigger status changes accordingly.

You can change this by adding one of the keywords present in the ignoreKeywords property in your Lunaria configuration file in the PR's title (ignoring all files) or by including a tracker directive in the merged commit's description.

Tracked Files

File Note
en/guides/integrations-guide/svelte.mdx Source changed, localizations will be marked as outdated.
Warnings reference
Icon Description
🔄️ The source for this localization has been updated since the creation of this pull request, make sure all changes in the source have been applied.

bluwy
bluwy commented Nov 13, 2024
View reviewed changes
src/content/docs/en/guides/integrations-guide/svelte.mdx
@@ -103,44 +103,28 @@ To use your first Svelte component in Astro, head to our [UI framework documenta

This integration is powered by `@sveltejs/vite-plugin-svelte`. To customize the Svelte compiler, options can be provided to the integration. See the [`@sveltejs/vite-plugin-svelte` docs](https://github.com/sveltejs/vite-plugin-svelte/blob/HEAD/docs/config.md) for more details.

### Default options
Copy link
Member Author

Choose a reason for hiding this comment

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

Note there are now no default options set by Astro. It falls back to the behaviour in @sveltejs/vite-plugin-svelte by default now.

Which means the notes about vitePreprocess are no longer relevant, the user will have to always add that themselves if they use scss/stylus etc. Since astro add svelte has been adding those for users by default for a long time, I don't think this will break a lot of folks. I've noted this in the core PR changeset

src/content/docs/en/guides/integrations-guide/svelte.mdx
Comment on lines -143 to +127
If you're using a preprocessor like TypeScript or SCSS in your Svelte files, you can create a `svelte.config.js` file so that the Svelte IDE extension can correctly parse the Svelte files.
If you're using SCSS or Stylus in your Svelte files, you can create a `svelte.config.js` file so that they are preprocessed by Svelte, and the Svelte IDE extension can correctly parse the Svelte files.
Copy link
Member Author

Choose a reason for hiding this comment

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

I removed references to TypeScript here as Svelte 5 now handles TypeScript by default even without vitePreprocess.

sarah11918
sarah11918 reviewed Nov 13, 2024
View reviewed changes
Copy link
Member

@sarah11918 sarah11918 left a comment

Choose a reason for hiding this comment

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

Thanks, looks great @bluwy ! I only had one question below, but otherwise good to merge when the feature is released! 🙌

src/content/docs/en/guides/integrations-guide/svelte.mdx
@@ -9,7 +9,7 @@ i18nReady: true
import PackageManagerTabs from '~/components/tabs/PackageManagerTabs.astro';
import Since from '~/components/Since.astro';

This **[Astro integration][astro-integration]** enables server-side rendering and client-side hydration for your [Svelte](https://svelte.dev/) components. It supports Svelte 3, 4, and 5 (experimental).
This **[Astro integration][astro-integration]** enables server-side rendering and client-side hydration for your [Svelte](https://svelte.dev/) 5 components. For Svelte 3 and 4 support, install `@astrojs/svelte@5` instead.
Copy link
Member

Choose a reason for hiding this comment

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

For Svelte 3 and 4 support, install @astrojs/svelte@5 instead

Just calling attention to it because it looks weird. Svelte 5 is the default. If you want 3 and install svelte@**5** -- just checking this isn't a typo or something!

Copy link
Member Author

Choose a reason for hiding this comment

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

It's referring to install @astrojs/svelte, which is an integration. And @astrojs/svelte v5 supports Svelte 3 and 4 only.

Svelte 5 support is added on @astrojs/svelte v6, which I didn't mention v6 here since it will be the latest.

Copy link
Member

Choose a reason for hiding this comment

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

Yup, figured this out in the other PR. What a pain, but understood!

Would be much nicer if our numbers aligned with theirs!

Copy link
Contributor

Choose a reason for hiding this comment

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

Would be much nicer if our numbers aligned with theirs!

Easy, just don't ship @astrojs/svelte v6 and update v5 until Svelte v6 drops. :trollface:

@sarah11918
Copy link
Member

Oh! just calling out that we should have something in the v5 upgrade guide, too! Let's get that in this PR so it doesn't get forgotten!

@bluwy
Copy link
Member Author

bluwy commented Nov 13, 2024

I suppose since this isn't related to Astro v5 we don't need to update the migration guide now? Currently the migration guide for @astrojs/svelte itself is in its changelog.

@sarah11918
Copy link
Member

Yup, that's when I thought this was related to Astro 5, too! This one's good to go!

sarah11918
sarah11918 approved these changes Nov 13, 2024
View reviewed changes
Copy link
Member

@sarah11918 sarah11918 left a comment

Choose a reason for hiding this comment

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

Ready to merge when the feature does!

@sarah11918 sarah11918 added the Merge Queue Approved and ready to be merged (wait for feature release if also labelled M-O-R)! label Nov 13, 2024
casungo
casungo approved these changes Nov 13, 2024
View reviewed changes
Copy link
Contributor

@casungo casungo left a comment

Choose a reason for hiding this comment

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

LGTM!

@sarah11918 sarah11918 merged commit eb0795f into main Nov 15, 2024
10 checks passed
@sarah11918 sarah11918 deleted the svelte-v6 branch November 15, 2024 11:51
@jsparkdev jsparkdev mentioned this pull request Nov 15, 2024
Merged
@Nin3lee Nin3lee mentioned this pull request Nov 18, 2024
Merged
@trueberryless trueberryless mentioned this pull request Nov 19, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@casungo casungo casungo approved these changes

@sarah11918 sarah11918 sarah11918 approved these changes

Assignees
No one assigned
Labels
improve or update documentation Enhance / update existing documentation (e.g. add example, improve description, update for changes) Merge Queue Approved and ready to be merged (wait for feature release if also labelled M-O-R)! merge-on-release Don't merge this before the feature is released! (MQ=approved but WAIT for feature release!)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@bluwy @astrobot-houston @sarah11918 @casungo