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

[Bug] type issue in addEvent #4504

Open
2 tasks done
lovasoa opened this issue Jan 16, 2025 · 0 comments
Open
2 tasks done

[Bug] type issue in addEvent #4504

lovasoa opened this issue Jan 16, 2025 · 0 comments
Assignees
Labels
bug Something isn't working

Comments

@lovasoa
Copy link

lovasoa commented Jan 16, 2025

Before you submit this issue, have you checked the following

  • Is this really a problem?
  • I have searched the Github Issues for similar issues, but did not find anything.

Affected packages and versions

latest

Reproduction link

The current type definition for addEvent has a potential type safety issue:

addEvent(event: keyof IEventParamConfig, callback: (params: IEventParamConfig[typeof event]) => void) {

The issue lies in using typeof event within the indexed access type. Since event is a parameter, typeof event resolves to just keyof IEventParamConfig, instead of the actual literal type used by the caller.

Image

Proposed Solution

We can improve type safety by using a generic parameter to preserve the literal type:

addEvent<E extends keyof IEventParamConfig>(
    event: E, 
    callback: (params: IEventParamConfig[E]) => void
): IDisposable;

which results in:

Image

Benefits

  1. Preserves the literal type of the event parameter through the generic constraint
  2. Provides better type inference for the callback parameter
  3. Maintains the same runtime behavior while improving compile-time type checking

Example Usage

// Before - Less precise typing
univerAPI.addEvent('DocCreated', (params) => {
    // params type is less specific
});

// After - Precise typing
univerAPI.addEvent('DocCreated', (params) => {
    // params is correctly typed to the specific event type
    params.unitId; // properly typed
    params.doc;    // properly typed
});

Expected behavior

univerAPI.addEvent('CommandExecuted', ({ id, params }) => {})

should work

Actual behavior

univerAPI.addEvent('CommandExecuted', ({ id, params }) => {})

raises

Property 'id' does not exist on type 'ILifeCycleChangedEvent | IDocDisposedEvent | IDocCreatedParam | ICommandEvent | ISheetCreatedEventParams | ... 10 more ... | ICellEventParam'.ts(2339)

System information

No response

@lovasoa lovasoa added the bug Something isn't working label Jan 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants