Skip to content

Conversation

@dummdidumm
Copy link
Member

closes #14900

@changeset-bot
Copy link

changeset-bot bot commented Jan 7, 2025

⚠️ No Changeset found

Latest commit: 2cee806

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@Rich-Harris
Copy link
Member

preview: https://svelte-dev-git-preview-svelte-14925-svelte.vercel.app/

this is an automated message

@github-actions
Copy link
Contributor

github-actions bot commented Jan 7, 2025

Playground

pnpm add https://pkg.pr.new/svelte@14925

@dummdidumm
Copy link
Member Author

I don't understand why this fails the deployment with "has implicit any" errors, the code is correct - what am I missing?

get value() {
return count * mult;
},
/** @param {number} c */
Copy link
Member

Choose a reason for hiding this comment

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

The JSDoc -> TS conversion code here doesn't know how to deal with annotated methods, only function and variable declarations. We'll need to update it

Copy link
Member Author

Choose a reason for hiding this comment

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

Adjusted that, still fails - turns out it's because of https://github.com/sveltejs/svelte.dev/blob/main/packages/site-kit/src/lib/markdown/renderer.ts#L702 where lang is always 'ts' - I have no idea how this worked until now, or if only by chance TS did fine so far. What was the reason for switching to hard-coding that as part of the shiki update in #288?

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 guess it's because twoslash does not run on JS files by default - will workaround it

@dummdidumm
Copy link
Member Author

Mhm the preview still fails, looks like it does not incorporate changes from main once it created a branch for a PR - guess we just YOLO-merge it and see what happens.

@Rich-Harris
Copy link
Member

It's still yikes-ing locally, I get this error:

Language ts not found, you may need to load it first

@Rich-Harris
Copy link
Member

Huh I made some local changes, then reverted them, and then it built successfully? idk computers are weird

@Rich-Harris Rich-Harris merged commit a1f371e into main Jan 7, 2025
11 checks passed
@Rich-Harris Rich-Harris deleted the test-docs branch January 7, 2025 17:06
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

Successfully merging this pull request may close these issues.

Improve documentation or example for vitest

3 participants