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

Ability to extend static Event interface #22946

Closed
wants to merge 1 commit into from

Conversation

Busyrev
Copy link
Contributor

@Busyrev Busyrev commented Mar 28, 2018

Main idea

Moved EventConstructor to separate interface for better extensibility.
Before this patch Event was declared with literal:

declare var Event: {
    prototype: Event;
    new(typeArg: string, eventInitDict?: EventInit): Event;
    readonly AT_TARGET: number;
    readonly BUBBLING_PHASE: number;
    readonly CAPTURING_PHASE: number;
};

in that case there is no ability to add static properties to Event using declaration merging.

Moved EventConstructor to separate named interface, now anyone can extend it in own project using declaration merging, same idea as ArrayConstructor declaration:

interface EventConstructor {
    prototype: Event;
    new(typeArg: string, eventInitDict?: EventInit): Event;
    readonly AT_TARGET: number;
    readonly BUBBLING_PHASE: number;
    readonly CAPTURING_PHASE: number;
}

declare var Event: EventConstructor;

As far as I know, this change made no side effects for anyone.

And some additional questions

1. Strange changes in dom.generated.d.ts

src\lib\dom.generated.d.ts git shows that 1144 lines added and 1143 removed, but TortoiseGitMerge shows only some lines changed(event declaration). Github shows many lines changed. I`m using Windows 10 may be it is line ending problem, but file is autogenerated, what should i do to make diff clear?

2. const vs var

Array declared:

declare const Array ...

But Event declared:

declare var Event ...

Should all declarations be const or var?

3. Different declarations fo Event in different files

Declaration of Event declared in tests\lib\lib.d.ts:

declare var Event: {
    prototype: Event;
    new(type: string, eventInitDict?: EventInit): Event;
    AT_TARGET: number;
    BUBBLING_PHASE: number;
    CAPTURING_PHASE: number;
}

differs from Event declared in all another d.ts files:

declare var Event: {
    prototype: Event;
    new(typeArg: string, eventInitDict?: EventInit): Event;
    readonly AT_TARGET: number;
    readonly BUBBLING_PHASE: number;
    readonly CAPTURING_PHASE: number;
};

shoul this declarations be equal? By meaning they are equal now.

4. Convert another declarations?

Would it be better to convert all delare var/const declarations with literal type to separate named types?

May be some questions better to convert to separate issues for discussion?

@Busyrev Busyrev changed the title Ability to extend static Event intreface Ability to extend static Event interface Mar 28, 2018
@mhegazy
Copy link
Contributor

mhegazy commented Mar 28, 2018

Thanks for your PR. these files are not manually edited, they are auto-generated from a script in . You can find more information about contributing lib.d.ts fixes at https://github.com/Microsoft/TSJS-lib-generator.

Changes should be done in https://github.com/Microsoft/TSJS-lib-generator instead.

Please find more information at: https://github.com/Microsoft/TypeScript/blob/master/CONTRIBUTING.md#contributing-libdts-fixes.

For future references, please file an issue first to track changes.

@mhegazy mhegazy closed this Mar 28, 2018
@mhegazy
Copy link
Contributor

mhegazy commented Mar 28, 2018

  1. Strange changes in dom.generated.d.ts

the files are auto generated.

  1. const vs var

var is used for back compat purposes, to allow redeclaration of the variable. we do not expect Array would be redefined, but other HTML* elements are frequently redefined in DT for new proposals that are not in the lib, e.g. WebGL, WebAudio, etc..

  1. Different declarations fo Event in different files

The files are generated by concatenating multiple sources.

  1. Convert another declarations?

No, creating new types pollute the global namespace, and we would like to keep these to a minimum

@mhegazy
Copy link
Contributor

mhegazy commented Mar 28, 2018

More importantly, we need some justification for adding the new type. what is the scenario you are trying to achieve, and why it is not working today. Please file an issue and these scenarios in it.

@microsoft microsoft locked and limited conversation to collaborators Jul 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants