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 #208

Closed
wants to merge 259 commits into from

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 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 attempting 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 what having very long and
    complicated if/else statements.

What is not included

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

@xeho91 xeho91 requested a review from JReinhold September 5, 2024 07:50
@xeho91 xeho91 assigned xeho91 and unassigned JReinhold Sep 5, 2024
@xeho91 xeho91 added the internal Changes only affect the internal API label Sep 5, 2024
@xeho91
Copy link
Collaborator Author

xeho91 commented Sep 5, 2024

Apologies, wrong --head branch used (used GitHub CLI), and I can't edit it 😭

#209

@xeho91 xeho91 closed this Sep 5, 2024
@xeho91 xeho91 removed the request for review from JReinhold September 5, 2024 07:54
@xeho91 xeho91 added duplicate This issue or pull request already exists and removed internal Changes only affect the internal API labels Sep 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate This issue or pull request already exists
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants