svelte icon indicating copy to clipboard operation
svelte copied to clipboard

fix: ensure createEventDispatcher works with types from generics

Open dummdidumm opened this issue 1 year ago • 2 comments

fixes #8860

This contains a small but unfortunately unavoidable breaking change: If you used never to type that the second parameter shouldn't be set (which the docs recommended for a short time), then you need to change that to null:

const dispatch = createEventDispatcher<{
-  noParameter: never;
+  noParameter: null;
}>();

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

  • [x] 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
  • [x] Prefix your PR title with feat:, fix:, chore:, or docs:.
  • [x] This message body should clearly illustrate what problems it solves.
  • [x] Ideally, include a test that fails without this PR but passes with it.

Tests and linting

  • [x] Run the tests with pnpm test and lint the project with pnpm lint

dummdidumm avatar Jun 28 '23 14:06 dummdidumm

We'll need to do the same for the Action interface 😞 will get to it later

dummdidumm avatar Jun 28 '23 15:06 dummdidumm

Hey! I observed a few interesting behaviors regarding the new createEventDispatcher type.

  1. This pull request slightly changes the behavior for null and undefined.

    // Setup
    const dispatch = createEventDispatcher<{
    	optional?: number;
    	nullable: number | null;
    }>();
    

    Before

    // Both are ok
    dispatch('optional');
    dispatch('nullable');
    
    // Both are ok
    dispatch('optional', undefined);
    dispatch('nullable', undefined);
    
    // Inconsistency
    dispatch('optional', null); // Error
    dispatch('nullable', null); // Ok
    

    After

    // All ok
    dispatch('optional');
    dispatch('nullable');
    dispatch('optional', undefined);
    dispatch('nullable', undefined);
    dispatch('optional', null);
    dispatch('nullable', null);
    
  2. As CustomEvent.detail defaults to null, the inferred types are slightly off when using undefined/?::

    const dispatch = createEventDispatcher<{ click: undefined }>();
    ```	
    
    ![image](https://github.com/sveltejs/svelte/assets/48261497/1bb3b7f7-4fe0-40ae-a57f-9ced0d62ff96)
    
    ![image](https://github.com/sveltejs/svelte/assets/48261497/703f2d41-e0d5-4468-93a3-2cd15c2fa92a)
    
    https://svelte.dev/repl/147bfc07869f4909b34f83951665155b?version=4.0.0
    
    Tracked in https://github.com/microsoft/TypeScript-DOM-lib-generator/issues/1585
    
    

I thought it was interesting to mention here, as tightening types might be a breaking change

GauBen avatar Jun 28 '23 17:06 GauBen

Thanks for testing these out! The new behavior is actually more correct, as undefined and null will both result in null for event.detail

The last insight is actually unrelated, it's how the Svelte language server resolves these types - that hasn't changed, but should probably adjusted to be null for detail in that case.

dummdidumm avatar Jun 28 '23 20:06 dummdidumm