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

refactor: Improve AST-related types readability & fix existing issues #209

Merged
merged 5 commits into from
Sep 5, 2024

Conversation

xeho91
Copy link
Collaborator

@xeho91 xeho91 commented Sep 5, 2024

To summarize, the main objective - fix CI check

What has been done

  1. Update imports & usage for types related to AST nodes.
    I've created two types (namespaces), SvelteAST & ESTreeAST that are used across entire codebase (imported from #parser/ast).
    I figured when wrapped into two namespaces is it easier to read and to quickly see from which AST they come from.
    This project is unique for using both Svelte AST and ESTree AST, so I hope this clears possible future confusion.
  2. After updating svelte to latest rc version (where AST types from svelte/compiler are finally exposed),
    I could see the existing types issues related to previous PR attempts to fix breaking change in Svelte AST
    (references: [Bug]: autodocs using svelte-csf v5 does not render individual story within docs #199 fix(parser): Resolve autodocs tag issue and extracting rawCode #201). So, I solved them + refactored them a little bit with early returns. There are small logic duplications, but I didn't mind them as they are easier to read and maintain than having very long and complicated if/else statements.

What is not included

There are only warnings left with deprecations related to <svelte:component> and <script context="module">.
I'd rather them being solved them in separate PRs.

@xeho91 xeho91 added the internal Changes only affect the internal API label Sep 5, 2024
@xeho91 xeho91 self-assigned this Sep 5, 2024
@xeho91
Copy link
Collaborator Author

xeho91 commented Sep 5, 2024

Oops, I broke build. Working on it

@xeho91
Copy link
Collaborator Author

xeho91 commented Sep 5, 2024

Blocked by: sveltejs/svelte#13135

@xeho91 xeho91 merged commit 1732cd7 into next Sep 5, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal Changes only affect the internal API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants