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

feat: first steps for adding switch-case blocks #7655

Draft
wants to merge 3 commits into
base: svelte-4
Choose a base branch
from

Conversation

MathiasWP
Copy link
Contributor

@MathiasWP MathiasWP commented Jul 3, 2022

{#switch} {:case} syntax

There has been discussions about adding switch case syntax to svelte (#530), and Rich Harris himself discussed how it could look like in his latest video Annoying things about svelte.

I tried to keep things simple when implementing this. I see the switch-case syntax as sugar for if-else, so it is definitely a goal to re-use the existing if-else logic instead of introducing a lot of new code.

This PR contains the branch I've been working on for the last couple of days, and it has the following:

1. parser implementation

I've implemented a SwitchBlock and CaseBlock node and the necessary logic in the parser to read the following syntax and produce an AST:

{#switch foo}
	I am default!
{:case "bar"}
	I am bar!
{/switch}

and

{#switch foo case "bar"}
	I am bar!
{:case "burger"}
	I am burger!
{/switch}

I've written tests for both cases:

I've structured the SwitchBlock with the following interface...

interface SwitchBlock {
	type: 'SwitchBlock';
	cases: CaseBlock[];
	discriminant: Expression;
	scope: TemplateScope;
}

...and the CaseBlock with the following interface

interface CaseBlock {
	type: 'CaseBlock';
	is_default: boolean;
	test?: Expression;
	children?: any[]
}

The SwitchBlock has a discriminant value, which is the value that the cases depend on. The CaseBlock can either be a default case or have a test defined. If it is the default case, then it should be added last in the list of the branches for the SwitchBlock. If it has a test, then it can be used to create a condition with the discriminant.

2. refactoring existing if-else logic in render_dom

I saw that the IfBlockWrapper class used IfBlockBranch classes to define what should be rendered, so i wanted to try and refactor these concepts and make them more generic. So i extracted these classes from the IfBlockWrapper and created two new classes, ConditionalBlockWrapper and ConditionalBlockBranch. Now the IfBlockWrapper and SwitchBlockWrapper wrappers both extend the ConditionalBlockWrapper to share the rendering functionality.

hitting the wall...

I want to manipulate the expression of a case-branch so that it goes from being

case_block.test // E.g. "bar" if the test-node was a "Literal" node

to

switch_block.discriminant === case_block.test

This is pretty much the only difference between if-else and switch-case blocks. If-else blocks already have the condition in place, while the case-blocks need to manipulate their expressions so that they add the strict equality operator to the discriminant.

However, i am not sure on what layer i should do this, and i also don't know how to set up dependencies correctly. I feel that this should be done in the Expression.manipulate method.

This is my current progress, but i am beginning to get stuck. I don't know all the in and outs of the Svelte codebase, and a lot of the code for the if-else logic is kinda hairy. I would appreciate any guidance on how to achieve this, and also feedback if the current progress is wrong.

todo

This list is probably incomplete, but these are the next steps that i know of:

  • Finish render_dom logic
  • SSR
  • Hydration
  • Better error messages
  • Language tools support
  • Documentation/Tutorial

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

@MathiasWP MathiasWP changed the title [feat]: First steps for adding {#switch} {:case} syntax [feat]: First steps for adding {#switch} {:case} blocks Jul 3, 2022
@MathiasWP MathiasWP changed the title [feat]: First steps for adding {#switch} {:case} blocks [feat]: First steps for adding switch-case blocks Jul 4, 2022
@tanhauhau
Copy link
Member

Hi @MathiasWP wow, you've made such great progress!

Thanks for picking this up and work on it. There's 1 huge todo that was missing in your list, which is to decide whether to introduce {#switch} as well as finalised on the syntax would be.

The one mentioned by rich in the video is more of a thinking-out-loud, where the syntax and such has not been finalized by the maintainers.

I'll help you bring this topic up to the maintainers again, and let you know the decision.

@MathiasWP
Copy link
Contributor Author

MathiasWP commented Jul 4, 2022

Hi @MathiasWP wow, you've made such great progress!

Thanks for picking this up and work on it. There's 1 huge todo that was missing in your list, which is to decide whether to introduce {#switch} as well as finalised on the syntax would be.

The one mentioned by rich in the video is more of a thinking-out-loud, where the syntax and such has not been finalized by the maintainers.

I'll help you bring this topic up to the maintainers again, and let you know the decision.

Hi @tanhauhau, thank you for the kind words!

I wanted to initiate the implementation of switch-case syntax, so i thought the best way would be to simply start, and perform changes along the way! I have no problem with discussing the syntax, i expected this to be honest. Creating the Nodes and parser logic was not difficult, so if we decide to change it then i have no issue changing it!

Thank you for taking this up with the maintainers! Looking forward to hearing what the decision is - no matter the outcome!

If it's okay, i'd like to add my thoughts to the syntax discussion. Rich mentioned that having a flat syntax like

{#switch foo}
	I am default!
{:case "bar"}
	I am bar!
{:case "baz"}
	I am baz!
{/switch}

was more "svelte-like", compared to the following syntax:

{#switch foo}
	{:case "bar"}
		I am bar!  
	{:case "baz"}
		I am baz!
	{:default}
		I am default!
{/switch}

However, i want to address a couple of things:

1. Switch-case statements are not flat by nature

If-else blocks are flat by nature:

if(foo) {
	return 'I am foo'
}
else if(bar) {
	return 'I am bar'
}
else {
	return 'I am default'
}

While switch-case blocks are not flat by nature:

switch (key) {
	case foo:
		return 'I am foo'

	case bar:
		return 'I am bar'

	default:
		return 'I am default'
}

Programmers are already used to switch-case blocks not being flat, so is it important to make them flat in Svelte? I personally think that it looks better when it is not flat.

2. Is it a good idea to change the behaviour of the switch statement?

I feel that it is a little scary moving the default statement to the top, only to keep the "flatness". Personally i believe that Svelte has done a good job at following existing and well known programming concepts and keeping things simple.

So why all of the sudden change the switch-case block to behave different than it naturally does?

@benmccann benmccann changed the title [feat]: First steps for adding switch-case blocks feat: first steps for adding switch-case blocks Feb 22, 2023
@dummdidumm dummdidumm added this to the one day milestone Feb 22, 2023
@baseballyama baseballyama marked this pull request as draft February 25, 2023 08:33
@adiguba
Copy link
Contributor

adiguba commented Dec 28, 2023

Just some remarks/idea about the proposed syntax.

  1. I dislike the fact that the default value is the first block of the switch.
    I don't find it very elegant, and it's contrary to the switch syntax in the majority of languages...
    I would rather have something like this :
{#switch foo}
{:case "bar"}
	I am bar!
{:case "baz"}
	I am baz!
{:default}
	I am default!
{/switch}

With some compile-time error :

  • There must be nothing between the {#switch} and the first {:case}.
  • There must be at most one {:default}, at the end of the block.
  1. {:case} block should accept multiple values :
{:case "bar", "foo"}
	I am bar or foo !
  1. {:case} block should accept state/variable :
{#switch foo}
{:case name}
	{foo} is equal (via ===) to {name}
  1. The compiler should detect duplicate literal value when possible
{#switch foo}
{:case "bar", "foo"}
	I am bar or foo !
{:case "baz"}
	I am baz!
{:case "foo"}             # ERROR duplicate value "foo"
	I am foo!
{:default}
	I am default!
{/switch}
  1. The {:case} blocks will expect values to compare in order to compare the equality, but we could also use {:when} blocks (or {:if} ?) in order to manage more complex conditions, where _ will match the current value of the switch :
{#switch foo}
{:case "bar", "foo"}
	I am bar or foo !
{:case "baz"}
	I am baz!
{:when _.length > 20}
       I'm big (more than 20 chars)
{:default}
	I am default!
{/switch}

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.

None yet

4 participants