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

TS to JSDoc Conversion #8569

Closed
wants to merge 544 commits into from

Conversation

tcc-sejohnson
Copy link
Contributor

HEADS UP: BIG RESTRUCTURING UNDERWAY

The Svelte repo is currently in the process of heavy restructuring for Svelte 4. After that, work on Svelte 5 will likely change a lot on the compiler aswell. For that reason, please don't open PRs that are large in scope, touch more than a couple of files etc. In other words, bug fixes are fine, but feature PRs will likely not be merged.

Before submitting the PR, please make sure you do the following

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • Prefix your PR title with feat:, fix:, chore:, or docs:.
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.

Tests

  • Run the tests with npm test and lint the project with npm run lint

@vercel
Copy link

vercel bot commented May 9, 2023

@tcc-sejohnson is attempting to deploy a commit to the Svelte Team on Vercel.

A member of the Team first needs to authorize it.

@tcc-sejohnson tcc-sejohnson changed the base branch from master to version-4 May 9, 2023 05:30
@vercel
Copy link

vercel bot commented May 9, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
svelte-dev-2 ❌ Failed (Inspect) May 9, 2023 5:30am

src/runtime/internal/loop.js Outdated Show resolved Hide resolved
@dyc3
Copy link

dyc3 commented May 9, 2023

Hey, just curious: why is this change being made? I was looking around for an issue or something for discussion around this but I couldn't find anything.

@baseballyama
Copy link
Member

As a Svelte compiler developer, debugging without a build step greatly simplifies compiler development. Previously, debugging was complicated by the fact that we had to debug using the build step.
In addition, using JSDoc does not affect compiler’s development safety because the type is almost equivalent to TS.

Of course, Svelte developers (not compiler developers) will still be provided type definition files as now, so there will be no change for Svelte developers in terms of typing.

@msereniti
Copy link

If the problem is reducing tests execution time maybe it is much better to focus on picking a faster Typescript transpiler (like esbuild or swc) rather than switching codebase from one language to a two separated ones what can cause new troubles like code desynchronization and weak abstraction relations?

@baseballyama
Copy link
Member

If the problem is reducing tests execution time

No. often library developers use npm link to link userland applications and debug the library.
If the program is built at debug time, it will be difficult to find the function we are looking for. Also, e.g. when using the debugging function of VSCode, , it is necessary to debug with a built program, which makes it difficult to know where we are and we need to go back to original program every time when we want to fix something.

The main point at present is whether TS or JS + JSDoc is more comfortable to write.

@connor4312
Copy link

Happened to see this on Twitter during lunch. I maintain the VS Code JS debugger.

when using the debugging function of VSCode, , it is necessary to debug with a built program, which makes it difficult to know where we are and we need to go back to original program every time when we want to fix something.

It's an aside from the main PR, but I'm not entirely sure what you mean here. This should not exclude the ability to use alternative TS compilers--in fact, the js debugger itself is built with esbuild. The debugger should also handle runtime transpilers (like tsx) just fine.

One snag you might hit with npm link is that by default we don't follow sourcemaps in node_modules for performance reasons, but this can be adjusted by setting resourceSourceMapLocations: null (overriding its default) in your launch.json. Anyhow, if you're running into places where the debugger doesn't seem to work, please feel free to open an issue.

@Niclassg
Copy link

As someone who has no stake in Svelte nor actively uses it I would be very much interested in a blog post describing the difficulties of using TS and why JSDoc + JS is seen as a better alternative so I can broaden my knowledge and understanding.

@PuruVJ
Copy link
Collaborator

PuruVJ commented May 10, 2023

As someone who has no stake in Svelte nor actively uses it I would be very much interested in a blog post describing the difficulties of using TS and why JSDoc + JS is seen as a better alternative so I can broaden my knowledge and understanding.

I wrote this some 2 years ago, should be relevant even now https://puruvj.dev/blog/get-to-know-typescript--using-typescript-without-typescript

@dtinth
Copy link

dtinth commented May 10, 2023

As someone who has no stake in Svelte nor actively uses it I would be very much interested in a blog post describing the difficulties of using TS and why JSDoc + JS is seen as a better alternative so I can broaden my knowledge and understanding.

Here’s some context:

@dummdidumm
Copy link
Member

I'm locking this PR for now to not distract maintainers with this conversation which was had multiple times before. As a user of Svelte this has NO impact whatsoever on you, and rest assured that the Svelte code base keeps the same level of type safety with this change.

@sveltejs sveltejs locked as too heated and limited conversation to collaborators May 10, 2023
…src/compiler/compile/render_dom/wrappers/shared/bind_this.js
…_comment.ts to src/compiler/compile/render_dom/wrappers/shared/create_debugging_comment.js
…ion.ts to src/compiler/compile/render_dom/wrappers/shared/get_slot_definition.js
… src/compiler/compile/render_dom/wrappers/shared/is_dynamic.js
…bindings.ts to src/compiler/compile/render_dom/wrappers/shared/mark_each_block_bindings.js
- generate-types script: deal with default imports
- store: private/public types split
- SvelteComponentDev/Typed: remove need for interface
@dummdidumm
Copy link
Member

For some reason GitHub fucked up and didn't mark this as merged into #8480, so I'm going to close this

@dummdidumm dummdidumm closed this May 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants