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

Reduce symbol instantiations in lib.d.ts #166

Merged
merged 6 commits into from
Nov 22, 2016
Merged

Conversation

mhegazy
Copy link
Contributor

@mhegazy mhegazy commented Nov 18, 2016

A few changes:

  • avoid using this types and instead add the overloads to every derived interface
  • use Mapped types instead of listing event names/tag names explicitly in overloads

//cc: @zhengbli

Build diagnostics building dom.genenrated.d.ts before the change:

Files:            2
Lines:        18814
Nodes:       102909
Identifiers:  35546
Symbols:      95889
Types:        12369
Memory used: 94151K
I/O read:     0.00s
I/O write:    0.00s
Parse time:   0.36s
Bind time:    0.18s
Check time:   1.13s
Emit time:    0.02s
Total time:   1.68s

After the change:

Files:            2
Lines:        17559
Nodes:        69571
Identifiers:  23524
Symbols:      18183
Types:         4095
Memory used: 52984K
I/O read:     0.01s
I/O write:    0.00s
Parse time:   0.32s
Bind time:    0.15s
Check time:   0.50s
Emit time:    0.02s
Total time:   0.98s

@rictic
Copy link
Contributor

rictic commented Nov 19, 2016

This is so cool! It looks like this will be great news for custom elements. e.g. experimenting with the nightly build and some of the example code in microsoft/TypeScript#12311 it seems like this typechecks:

class MySlider extends HTMLElement {
  slide() {}
}
customElements.define('my-slider', MySlider);
interface HTMLKind {
  'my-slider': MySlider;
}

document.createElement('my-slider').slide()

So awesome!


let implementedEventHandler =
let implementis = i.Implements |> Array.map GetInterfaceByName
[ for i' in implementis do
yield! match i' with
| Some i -> GetEventHandler i
| Some i -> if hasHandler i then [i] else []
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this returns the interface itself instead of the handlers it has?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes. i could make it the interface name. but seemed the same

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then the name of the variable should be changed to implementedInterface and other places, otherwise it can be confusing later


let EmitElementTagNameMap () =
Pt.printl "interface ElementTagNameMap {"
Pt.increaseIndent()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like ElementTagNameMap interface is always the same as HTMLElementTagNameMap? Then make is an alias instead

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no they are different. not sure why though. i wanted to ask you about that.

Copy link
Contributor

@zhengbli zhengbli Nov 21, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aha now I remember, the EmitHTMLElementTagNameMap only prints tag elements that either extends or implements the HTMLElement type.

Pt.printl "interface ElementListTagNameMap {"
Pt.increaseIndent()
for e in tagNameToEleName do
Pt.printl "\"%s\": NodeListOf<%s>;" (e.Key.ToLower()) e.Value
Copy link
Contributor

@zhengbli zhengbli Nov 21, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can do

type ElementListTagNameMap = { [K in ElementTagNameMap]: NodeListOf<ElementTagNameMap[K]> }

and avoid the loop

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is what i had earlier, then changed it. the issue is that NodeListOf has a constraint that T extends Node. we can not verify this constraint on ElementTagNameMap[K] unless i add a string indexer. adding the indexer makes the language service experience of the signature help much worse. so this is a work around for that.

Pt.print " {"
Pt.increaseIndent()
ownEventHandles |> List.iter EmitInterfaceEventMapEntry
// Pt.printl "[x: string]: Event;"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove this

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

@zhengbli
Copy link
Contributor

The addEventListener in the addedTypes.json file need to be changed as well, and other than that looks good to me. Thanks!

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.

4 participants