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

fix: error at compile time on unsupported TypeScript language features #12982

Merged
merged 8 commits into from
Sep 3, 2024

Conversation

Rich-Harris
Copy link
Member

@Rich-Harris Rich-Harris commented Aug 22, 2024

part of #11502 - to close it completely, we also need to look at using and possibly implement heuristics within bundler plugins to give more details

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 and linting

  • Run the tests with pnpm test and lint the project with pnpm lint

Copy link

changeset-bot bot commented Aug 22, 2024

🦋 Changeset detected

Latest commit: 8f44b58

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
svelte Patch

Not sure what this means? Click here to learn what changesets are.

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

@Rich-Harris
Copy link
Member Author

Rich-Harris commented Aug 22, 2024

It seems that acorn-typescript doesn't parse enums using (we should probably open an issue), while acorn does parse decorators but esrap doesn't print them (we should probably open an issue).

What other language features do we need to consider?

@dummdidumm
Copy link
Member

dummdidumm commented Aug 22, 2024

  • constructor(private/public/protected x)
  • namespace X { export function y() { ... } } (though it seems we don't strip namespaces that only consist of types currently, and neither do we strip declare module, nor declare const/function/etc, which we should)
  • class X { accessor y = .. }
  • using ... (because it's not stage 4 yet / not in acorn)

@Rich-Harris
Copy link
Member Author

oh wait i mean using instead of enums above

@Rich-Harris
Copy link
Member Author

Also I misspoke when I said Acorn parses decorators — it's actually acorn-typescript doing that.

There is an effort underway to add TS support to esrap, which would include decorators and everything else. If that landed, would it in fact make sense to just support the features in question?

@dummdidumm
Copy link
Member

dummdidumm commented Aug 23, 2024

I mean theoretically yes, but it would certainly be a non-trivial effort. The transformations that TS does under the hood are somewhat involved (you can look at the JS output tab in their playground).

@Rich-Harris
Copy link
Member Author

I'm not suggesting we transform TypeScript syntax, just that we understand it enough that it doesn't mess up the analysis, and then we emit TS rather than JS

@dummdidumm
Copy link
Member

Oh so you're saying that whatever comes after Svelte has to then know "ok this is a typescript file now so I'm gonna transform it using esbuild/tsc/whatever"? @dominikg how would that impact the toolchain?

@dominikg
Copy link
Member

allowing the svelte compiler to emit ts that then has to be postprocessed is going to create a lot of effort. even if it works in vite, what about all the other bundlers or even just our repl?

@Rich-Harris
Copy link
Member Author

Unfortunately this is the kind of thing the bundler designers of the world didn't anticipate — everything is more or less based on file extensions, so you'd need to configure the downstream transformers to keep an eye out for .svelte files as well.

Have thought for a while that the ecosystem would be more robust if instead of just returning { code, map } we could return various well-known attributes like language/version/whatever, so that a TS plugin could say 'oh, this .potato file generated some TypeScript — guess I'm up'. Hindsight is 20/20.

Copy link
Member

@dummdidumm dummdidumm left a comment

Choose a reason for hiding this comment

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

I added validation for all features we can validate (i.e. everything that came to my mind except using which ts-acorn doesn't parse yet). This should be a good stop gap solution until we figure out the next steps (or decide that this is the end state, which I would also be good with, given that changing toolchains is a big effort)

@dummdidumm dummdidumm merged commit 194570d into main Sep 3, 2024
9 checks passed
@dummdidumm dummdidumm deleted the gh-11502 branch September 3, 2024 09:01
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.

3 participants