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

Add es3.d.ts #16077

Closed
wants to merge 9 commits into from
Closed

Add es3.d.ts #16077

wants to merge 9 commits into from

Conversation

saschanaz
Copy link
Contributor

@saschanaz saschanaz commented May 25, 2017

Fixes #2410
Fixes #15978

Test failure should be fixed after ivogabe/gulp-typescript#510 is merged. Done

Gulpfile.ts Outdated
{ target: "lib.es5.d.ts", sources: ["header.d.ts", "es5.d.ts"] },
{ target: "lib.es2015.d.ts", sources: ["header.d.ts", "es2015.d.ts"] },
{ target: "lib.es2016.d.ts", sources: ["header.d.ts", "es2016.d.ts"] },
{ target: "lib.es2017.d.ts", sources: ["header.d.ts", "es2017.d.ts"] },
{ target: "lib.esnext.d.ts", sources: ["header.d.ts", "esnext.d.ts"] },

// JavaScript + all host library
{ target: "lib.d.ts", sources: ["header.d.ts", "es5.d.ts"].concat(hostsLibrarySources) },
{ target: "lib.d.ts", sources: ["header.d.ts", "es3.d.ts"].concat(hostsLibrarySources) },
{ target: "lib.es5.full.d.ts", sources: ["header.d.ts", "es5.d.ts"].concat(hostsLibrarySources) },
Copy link
Contributor

Choose a reason for hiding this comment

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

i would keep lib.d.ts to be es5 and create lib.es3.full just to avoid breaking users how depend on a specific meaning of lib.d.ts.

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. (It was intended to suppress baseline changes.)

@mhegazy
Copy link
Contributor

mhegazy commented May 25, 2017

@rbuckton mind taking a look at the lib changes.

@mhegazy mhegazy requested a review from rbuckton May 25, 2017 19:31
@saschanaz
Copy link
Contributor Author

saschanaz commented May 26, 2017

I included typed arrays in es3.d.ts as:

  1. it was already included in es5.d.ts even when it should be in es2015.d.ts
  2. excluding them will break DOM types which depends on typed arrays

We probably can move their constructors to es2015.d.ts, similar to what's done for Promises.

@ericdrobinson
Copy link

I included typed arrays in es3.d.ts as:

  1. it was already included in es5.d.ts even when it should be in es2015.d.ts
  2. excluding them will break DOM types which depends on typed arrays

We probably can move their constructors to es2015.d.ts, similar to what's done for Promises.

On TypedArrays in es3.d.ts

I would strongly request that the TypedArray types be moved to a different location. Without this adjustment, this would not completely fix #2410 as suggested in the initial post. Part of the purpose for adding an ES3-specific declaration file is to enable TypeScript's use in environments based on the ES3 language specification (like ExtendScript). One massive motivation for having an ES3-specific library in the first place is to help people determine what is and isn't supported in the target environment (see: this example for an ES6 equivalent). What is gained by allowing TypeScript to resolve UInt8Array in raw ES3 environments?

Going Further - Remove All Non-ES3 Types

I don't understand why the Promise type declarations made it into the ES3 declarations file*, but I would suggest that those remain in the ES5 file (at the very least). This goes with everything on this line and below, actually. None of those exist in the ES3 spec and, according to @mhegazy (emphasis mine):

@saschanaz you can always augment your program with additional types that you know would exist on your target system. for instance, if you know you are always running on an engine that supports promises, just add declarations for Promise in your program. The idea is that the lib file reflects the baseline of engines supporting the target of the compilation (ES5 v.s. ES6)

That Promises, TypedArrays, etc. exist in the ES5 lib at all is somewhat confusing as they do not exist in the ECMAScript 5.1 spec. It seems, in fact, that es5.d.ts inherited [many of?] those type declarations from its predecessor, core.d.ts. This has since been seemingly reinforced, for example, by moving the Promise<T> interface into es5.d.ts. I do not know why this has been maintained, rather than pulled into its own support library yet, but perhaps it's time.

* Actually, I have a guess. Your initial commit changed things such that lib.d.ts was based on es3.d.ts and no longer es5.d.ts. The es3.d.ts file would therefore need to somehow include those type declarations so as not to institute a breaking change. Perhaps a better change would be to move those types into a separate declaration file and then include it in the sources array for lib.d.ts?

@saschanaz
Copy link
Contributor Author

saschanaz commented May 29, 2017

@ericdrobinson Promise initially was not on es5.d.ts, it moved later by #14053.

The sole purpose of including Promise and Typed Arrays to prevent errors from lib.dom.d.ts where APIs e.g. Fetch and Web Audio use them. If the types are removed from es3 then users will get errors every time they build for ES3 target.

I think you won't be confused too much for Promise as the Promise constructor is intentionally only supported on ES2015+, which means you cannot do new Promise() on ES3.

@ericdrobinson
Copy link

@saschanaz

The sole purpose of including Promise and Typed Arrays to prevent errors from lib.dom.d.ts where APIs e.g. Fetch and Web Audio use them.

How is this not resolved by moving them to a separate type document that lib.dom.d.ts is aware of? You could specify this separately in the gulpfile. As an example, let's say everything was cleanly returned to es2015.promise.d.ts. Could you not, then, simply adjust the lib.dom.d.ts specification to use:

{ target: "lib.dom.d.ts", sources: ["header.d.ts", "dom.generated.d.ts"].concat("es2015.promise.d.ts") },

I do not know enough, currently, about the state of Promises to say whether or not this is the correct path or if a separate dom.promise.d.ts file should be made and referenced, rather than the es2015-specific version. However, this would allow for a clean es3 and/or es5 specification.

Am I missing something critical here?

@ericdrobinson
Copy link

I do not know enough, currently, about the state of Promises to say whether or not this is the correct path or if a separate dom.promise.d.ts file should be made and referenced, rather than the es2015-specific version. However, this would allow for a clean es3 and/or es5 specification.

I expanded upon this a bit in #16132.

@ericdrobinson
Copy link

I think you won't be confused too much for Promise as the Promise constructor is intentionally only supported on ES2015+, which means you cannot do new Promise() on ES3.

But this does not apply for TypedArrays, right? The es3.d.ts file includes constructors for those. I tested in current TypeScript by specifying es5-only ("lib": ["es5"]). You can end up in a situation where you assume that your code is correct to include UInt8Array as TypeScript will suggest it and happily turn your code into JavaScript. The resulting code won't run, however, in environments that truly only support ES5. Boo.

Pretty much everything below this line is merely included as a kludge to assist in supporting other, dependent libraries. This kludge is a hold-over from a time before the --lib compiler option existed, when all users had was the --target flag. Now that we have the ability to be more granular/explicit with environment setup, shouldn't we be? :)

@saschanaz
Copy link
Contributor Author

Could you not, then, simply adjust the lib.dom.d.ts specification to use:

I think it's somehow doable, but probably with another PR (as this one already has many baseline changes). I will talk more on #16132.

@ericdrobinson
Copy link

TL;DR - Could you at least move all types below this line back into the ES5 lib? That would leave the default setting (lib.d.ts) unchanged, while allowing more granular (and correct) environment configuration for users who need ES3...

I think it's somehow doable, but probably with another PR (as this one already has many baseline changes).

When you initially wrote the ES3 library you also changed it to be the default library imported by lib.d.ts. That has subsequently changed. With that change, a user wishing to use ES3 must specify as much using the lib flag in their tsconfig.json:

"lib": ["es3"]

For a user who wishes to also use DOM APIs, they would need to specify:

"lib": ["es3", "dom"]

I should clarify that this is decidedly not a problem. It works as expected.

However, now that ES5 has been switched back to being the default, the kludge of including the DOM support types can be moved back into "es5", right? What this would mean for someone wishing to use an ES3+DOM environment is that they would have to specify the following:

"lib": ["es3", "dom", "es2015.promise.d.ts"]

Such users would have access to the [decidedly bloated] DOM APIs but could still code with ES3. They would of course end up with Promises in their types, but this is a side-effect of using the monolithic DOM library, not an incorrectly formed ECMAScript type library.

By keeping the ES3 lib to types only specified in the ECMAScript 3 language specification, you can still have ES5 use it as a base and better support environments that are strictly ES3-based (including ExtendScript).

@saschanaz
Copy link
Contributor Author

With that change, a user wishing to use ES3 must specify as much using the lib flag in their tsconfig.json:

No, --target es3 will be sufficient. It's just that the default is now lib.es3.full.d.ts rather than lib.d.ts, where both includes DOM types.

Could you at least move all types below this line back into the ES5 lib?

As --target es3 gives DOM types by default it will break the compiler.

@ericdrobinson
Copy link

No, --target es3 will be sufficient. It's just that the default is now lib.es3.full.d.ts rather than lib.d.ts, where both includes DOM types.

Perhaps I don't understand how --target works. My understanding was that --target specified the output, whereas --lib specified the development environment (i.e. what types were identified by the compiler). This seems to bear out as we've been able to set --target es3 for quite a long time but have not had an actual es3 specific type library in the past.

As --target es3 gives DOM types by default it will break the compiler.

First, where is a good entrypoint in the codebase for understanding how --target is actually used? I would like to better understand it.

Second, if for whatever reason, compilation breaks when you don't include the Promise types, what is stopping you from specifying "es2015.promise.d.ts" for the "full" es3, but leave the base es3 alone?

For that matter, --target es3 has been working without the ES3-specific library. Did it previously use an es5 lib? If so, why not continue using it? No ES5-specific types would pass the type checker before making it into the compilation phase with the correct lib setup, no?

@saschanaz
Copy link
Contributor Author

First, where is a good entrypoint in the codebase for understanding how --target is actually used? I would like to better understand it.

ts.getDefaultLibFileName() is relevant. This function gives the default lib file name for each compilation target. --lib es3 will override it, as you know.

what is stopping you from specifying "es2015.promise.d.ts" for the "full" es3, but leave the base es3 alone?

If you look at the es2015.promise.d.ts it only contains Promise constructor type, not Promise instance type. Adding it on lib.es3.full.d.ts will unexpectedly allow new Promise() for ES3 targets.

Did it previously use an es5 lib? If so, why not continue using it?

Yes it used es5 lib, and will be changed because there were some valid requests for this.

@ericdrobinson
Copy link

ericdrobinson commented May 30, 2017

ts.getDefaultLibFileName() is relevant. This function gives the default lib file name for each compilation target. --lib es3 will override it, as you know.

Thanks. This is very helpful. Came just as I located this note in the code where it actually gets used. I feel I have a more complete understanding now.

If you look at the es2015.promise.d.ts it only contains Promise constructor type, not Promise instance type.

Ahh, yes, the "fix" for that is back over in #16132. Such a fix would require implementing (even if temporarily) one of the fixes suggested there.

Adding it on lib.es3.full.d.ts will unexpectedly allow new Promise() for ES3 targets.
This does not apply at code-time when the user specifies --lib es3 as the es3.d.ts file is used directly. This would only apply at compilation-time. By that point, Promise types would not have resolved and the user would have needed to fix them. In other words, you would not see auto-completion suggestions with Promise as a suggested type.

I understand that specifying --target es3 alone would bring in the Promise stuff. That is unfortunate and something that [hopefully] a combination of #16132, #4692, and #15529 (and others?) would resolve - enabling better management of global type scope and providing more granular DOM libraries. However, it is still possible to specify a development environment that does not include the dom types (--lib es3 --target es3). As the only issue here is that --target es3 alone brings in DOM, perhaps those kludge types should be moved into a "domSupportTypes.d.ts" file and included in libraries that currently rely upon it (e.g. es5.full.d.ts, lib.d.ts, and es3.full.d.ts). That would go a long way to helping #16132 and theoretically shouldn't do much more than create a simple, self-explanatory declaration file that clearly states its purpose for existence. This would also enable the es3 and es5 libraries to properly align with their respective language specifications, as theoretically the es6 library does.

@saschanaz
Copy link
Contributor Author

perhaps those kludge types should be moved into a "domSupportTypes.d.ts" file and included in libraries that currently rely upon it (e.g. es5.full.d.ts, lib.d.ts, and es3.full.d.ts).

I would name it lib.es2015.dom.d.ts and include it on es2015.d.ts (one of the library that relying on it).

@ericdrobinson
Copy link

I would name it lib.es2015.dom.d.ts and include it on es2015.d.ts (one of the library that relying on it).

That makes sense.

As much as it might feel "more correct" to move the offending type declarations into their respective es2015 libraries (e.g. Promise into es2015.promise.d.ts, TypedArrays into es2015.core.d.ts, Decorators into [some new proposals.d.ts?]) as suggested as a potential solution on #16132, shunting them into a lib.es2015.dom.d.ts would at least get them cordoned off into a dom-specific location and leave the core ECMAScript libs as a "stable" base.

/**
* Provides functionality common to all JavaScript objects.
*/
declare const Object: ObjectConstructor;
Copy link
Member

Choose a reason for hiding this comment

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

The ES3 spec does not declare these as const. Doing so would prevent shim libraries from being able to replace globals in TypeScript. Generally I think these should stay as var.

@DanielRosenwasser, do you have an opinion here?

new (byteLength: number): ArrayBuffer;
isView(arg: any): arg is ArrayBufferView;
}
declare const ArrayBuffer: ArrayBufferConstructor;
Copy link
Member

Choose a reason for hiding this comment

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

Typed Arrays are not part of ES3 or ES5. We included them in ES5 to support the DOM. One option to still support ES3 and not break the DOM lib would be to have the interfaces present in the ES3 lib, but leave the constructor declarations (e.g. declare var ArrayBuffer: ArrayBufferConstructor) in ES5 for now. That way, the types exist at design time but you cannot actually create an instance as there is no value declaration.

This is the approach we took for Promise. The interface for Promise was moved to ES5, but the value declaration was left in es2015.promise.

Choose a reason for hiding this comment

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

@rbuckton Did you perchance read over the comments on this topic in the thread? Would be interested in your thoughts as to why these shouldn't simply be moved to a separate document altogether that's included in the "full" versions of the libs, rather than the core ones. See this post and several preceding it.

Copy link
Member

Choose a reason for hiding this comment

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

I did see the thread, yes. We may eventually want to break those down into their own libs, but include them in the default lib in some fashion. I'd also like to be able to break down lib.dom.d.ts, but it is currently auto-generated and such a breakdown would be cumbersome at this point.

I've started investigating this as part of #15780, however that need not block this PR.

Choose a reason for hiding this comment

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

I'd also like to be able to break down lib.d.ts, but it is currently auto-generated and such a breakdown would be cumbersome at this point.

Would it really be that cumbersome? Wouldn't that simply involve adding a lib.es2015.dom.d.ts file to the src/lib directory and then adjusting the lib.d.ts (and others) to make use of it?

I've started investigating this as part of #15780

I take it that this would imply changing the system such that a lib.d.ts file actually existed within src/lib folder and used the new lib keyword to bring in the types, rather than compile it together as part of a build step?

Copy link
Member

Choose a reason for hiding this comment

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

I'd also like to be able to break down lib.d.ts, but it is currently auto-generated and such a breakdown would be cumbersome at this point.

Would it really be that cumbersome? Wouldn't that simply involve adding a lib.es2015.dom.d.ts file to the src/lib directory and then adjusting the lib.d.ts (and others) to make use of it?

You either misread or misquoted me. I did not say lib.d.ts would be cumbersome, but rather that lib.dom.d.ts would be as it is auto-generated. The tool that does this doesn't have a mechanism to break down features by which ECMAScript edition is supported. I don't know how much effort changing that would entail, but I do know that it is not insignificant.

I take it that this would imply changing the system such that a lib.d.ts file actually existed within src/lib folder and used the new lib keyword to bring in the types, rather than compile it together as part of a build step?

Roughly, yes. See src/lib/default.es5.d.ts

Choose a reason for hiding this comment

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

You either misread or misquoted me.

Yup. A little from column A; a little from column B, to be sure. Sorry about that!

I did not say lib.d.ts would be cumbersome, but rather that lib.dom.d.ts would be as it is auto-generated. The tool that does this doesn't have a mechanism to break down features by which ECMAScript edition is supported.

Ahh, I see. Perhaps, then, you misunderstood or misread the discussion above? We were not discussing the option of moving the TypedArray, Promise, etc. types back into lib.dom.d.ts, but rather to a separate support lib (lib.es2015.dom.d.ts or dom.support.d.ts or what-have-you); one who's purpose would be to provide the default "full" libraries with the necessary types. At the end of the day, this would result in no change to the default setup or the "full" libraries, only allowing the ES3 and ES5 libs to be, well, spec-compliant. The DOM/ECMAScript feature matrix should still be handles separately.

Please take a look at #16132. It details how many of the non-ECMAScript based "support" types came into the picture and when. The idea here is to move those types out of the (ex-core) ES3/ES5 context and into a separate "bandaid" library, which would allow ES3, ES5, etc. libraries to properly conform to the specs.

Part of the reason that I'm pushing for a move like this is that

  1. It does not appear to be difficult to arrange the code this way,
  2. it truly highlights an issue with the current setup and begins to isolate "problem code" such that a future fix will involve removing this "sore spot", and
  3. helps clean up the ES3 and ES5 libraries, which only contain several of those types because they initially evolved out of the original core.d.ts library.

Roughly, yes. See src/lib/default.es5.d.ts

I see. Given this new approach, it might then change the suggestion I put forth to effectively become:

/// <reference lib="dom.support" />

Perhaps it's not the cleanest, but it would theoretically isolate ES3 and ES5 better, making further edits to those libraries be maintenance-only and help curb (or enable workarounds for) the odd conflicts that show up occasionally.

Copy link
Member

Choose a reason for hiding this comment

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

I think I see what you are getting at. Perhaps the best breakdown would be:

  • lib.es3.d.ts - ES3 only
  • lib.es5.d.ts - ES5 only, though may need to delay several releases as node.d.ts depends on Promise/TypedArray as it exists today
  • lib.es2015.promise.core.d.ts - Promise type information (without Promise constructor)
  • lib.es2015.promise.d.ts - References es2015.promise.core, adds Promise constructor
  • lib.es2015.typedarray.core.d.ts - TypedArray types (without TypedArray constructors)
  • lib.es2015.typedarray.d.ts - References es2015.typedarray.core, adds TypedArray constructors
  • lib.dom.d.ts - References es2015.promise.core and es2015.typedarray.core, contains all of dom.generated

The lib.dom.d.ts would have to pull in Promise and TypedArray as they are required.

Choose a reason for hiding this comment

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

@rbuckton That would certainly be a clean approach! To where would you suggest moving the Decorator types? As far as JavaScript is concerned, these are still a proposal (perhaps even more of a reason to get them out of the EC3/EC5 libraries. Given that even in TypeScript they require the experimentalDecorators compiler option, perhaps a library like the following would work:

These could probably exist on their own, to be honest. They were initially brought into core before it became es5.d.ts and seem to have just stuck around. There appear to be no interdependencies with other types in either lib.d.ts or lib.dom.d.ts. They are used in TypeScript when the experimentalDecorators option is set, but perhaps that setting can also imply lib=["experimental.decorator"]?

@rbuckton
Copy link
Member

rbuckton commented Jun 6, 2017

Also, please resolve the current conflicts with master.

@mhegazy
Copy link
Contributor

mhegazy commented Jun 9, 2017

@saschanaz this change will need to wait for #15780 to go in first. #15780 will split lib.d.ts into smaller components, so this will avoid some of the conditions you have for when es3.full would be used.

@BurtHarris
Copy link

IMHO this is an unnecessary breaking change. See discussion in #16132.

@saschanaz
Copy link
Contributor Author

@BurtHarris This PR just adds es3.d.ts and won't break anything. #16132 (and some part of the conversation here) is a different discussion.

@NN---
Copy link

NN--- commented Dec 18, 2017

Any progress on this PR?
lib.es3.d.ts is definitely needed.

@saschanaz
Copy link
Contributor Author

@NN--- Blocked by #15780.

@typescript-bot
Copy link
Collaborator

Thanks for your contribution. This PR has not been updated in a while and cannot be automatically merged at the time being. For housekeeping purposes we are closing stale PRs. If you'd still like to continue working on this PR, please leave a message and one of the maintainers can reopen it.

@ericdrobinson
Copy link

@saschanaz With respect to this:

Blocked by #15780.

It appears as though @rbuckton completed the work (though that specific issue was superseded by #23893). Is the work done in that merge enough to get this moving?

@ecraig12345
Copy link
Member

Maybe a silly question but is it really necessary that the ES5+ type definitions be changed to extend the ES3 definitions? Seems like omitting that part would greatly simplify these code changes, unless it would break something down the line that I'm not aware of. (The other concern I can think of is that definitions shared between the ES3 and ES5 types could get out of sync.)

@saschanaz saschanaz mentioned this pull request Nov 6, 2018
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.

@lib: ES6 on fourslash test case breaks tests Create ES3 specific lib.d.ts
10 participants