Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

Declare SymbolConstructor for older versions of ES #7

Merged
merged 1 commit into from
Jun 3, 2019

Conversation

hausdorff
Copy link
Contributor

Older versions of the ES standard do not define Symbol#asyncIterator.
This means our attempts to "manually" define AsyncIterator for
versions of ES that don't have it fail.

Following 1 and 2, our understanding is that the best existing
solution is to manually re-define SymbolConstructor to have this
member.

Copy link
Member

@justinvp justinvp left a comment

Choose a reason for hiding this comment

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

LGTM (see pulumi/pulumi#2630 (comment)).

Related follow-up: I noticed this library is now targeting es5 and is setting downlevelIteration to true...

"downlevelIteration": true,

I think this library should be targeting es6 with downlevelIteration flag removed. All versions of Node.js we support works with es6, all our other Node.js libraries target es6, the default ts-node config for TypeScript projects targets es6, and our TypeScript templates target es6 by default.

Older versions of the ES standard do not define `Symbol#asyncIterator`.
This means our attempts to "manually" define `AsyncIterator` for
versions of ES that don't have it fail.

Following [1] and [2], our understanding is that the best existing
solution is to manually re-define `SymbolConstructor` to have this
member.

[1]: microsoft/TypeScript#13031
[2]: microsoft/TypeScript#8099
@hausdorff
Copy link
Contributor Author

@justinvp I meant for this to be also useful to non-Pulumi people, but we can bump to es6 and wait for people to ask for something else.

@hausdorff hausdorff merged commit eeb5ac4 into master Jun 3, 2019
@pulumi-bot pulumi-bot deleted the hausdorff/declare-symbol branch June 3, 2019 10:00
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.

2 participants