[NodeJS] Support 'typesVersions' in node/v6 through node/v9 #34678

rbuckton · 2019-04-12T00:36:57Z

Similar to the recent change to split NodeJS v11 and v10 to support "typesVersions", this does the same for NodeJS v9 and v8. This change will be necessary to continue to support Node v8 and v9 once microsoft/TypeScript#30790 has merged.

typescript-bot · 2019-04-12T01:22:40Z

@rbuckton Thank you for submitting this PR!

Because this is a new definition, a DefinitelyTyped maintainer will be reviewing this PR in the next few days once the Travis CI build passes.

In the meantime, if the build fails or a merge conflict occurs, I'll let you know. Have a nice day!

typescript-bot · 2019-04-12T01:22:41Z

@rbuckton The Travis CI build failed! Please review the logs for more information.

Once you've pushed the fixes, the build will automatically re-run. Thanks!

rbuckton · 2019-04-12T04:14:07Z

@sandersn: Any idea what's going on here? types-publisher is complaining about a package called aframe being added to notNeededPackages.json, but I don't see that package listed.

SimonSchick · 2019-04-12T19:02:29Z

@rbuckton with the size of node typings growing continuously, are there any plans to purge the EOL versions <= 6 (end of april) or even move them into a standard TS lib?

sandersn · 2019-04-16T00:41:25Z

@rbuckton The diff with master is quite complicated in that run. aframe shows up as partly deleted, which is what causes the error, but three (which aframe depends on) shows up as added. This makes me think that the branch must be fairly old (pre-March 27). I think that the notNeededPackages code I added recently gets confused when a popular package, with dependents, gets removed, causing dependents to add a package.json.

I would merge from master as a workaround.

typescript-bot · 2019-04-17T19:25:37Z

@rbuckton The Travis CI build failed! Please review the logs for more information.

Once you've pushed the fixes, the build will automatically re-run. Thanks!

typescript-bot · 2019-04-17T20:28:20Z

@rbuckton The Travis CI build failed! Please review the logs for more information.

Once you've pushed the fixes, the build will automatically re-run. Thanks!

rbuckton · 2019-04-17T20:58:30Z

@sandersn: Thanks, rebasing with latest master seems to have fixed the issue.

rbuckton · 2019-04-17T21:02:10Z

Since the bot things this is a new definition, I'll notify the reviewers manually:

🔔 @microsoft @DefinitelyTyped @jkomyno @a-tarasyuk @alvis @r3nya @btoueg @BrunoScheufler @smac89 @tellnes @Touffy @DeividasBakanas @eyqs @Flarna @Hannes-Magnusson-CK @KSXGitHub @hoo29 @kjin @ajafff @islishude @mwiktorczyk @matthieusieben @mohsen1 @n-e @octo-sniffle @parambirs @eps1lon @SimonSchick @ThomasdenH @WilcoBakker @wwwy3y3 @ZaneHannanAU @jeremiergz @samuela @kuehlein @j-oliveras @bhongy - please review this PR in the next few days. Be sure to explicitly select Approve or Request Changes in the GitHub UI.

Flarna · 2019-04-18T12:23:01Z

Node.Js 9 is EOL since 2018-06-30, long before Node.Js 6 (2019-04-30). Therefore I don't think changes v9 are needed.
We should consider to remove EOL versions from the repo instead to avoid references to it and people modifying it. Or at least mark them as frozen.

SimonSchick · 2019-04-18T13:36:04Z

@Flarna I think we should remove them, the node team doesn't maintain them and neither should we, people can always augment their typings.

rbuckton · 2019-04-18T18:08:10Z

The split of Node v8 and Node v9 typings has less to do with maintainability, and more to do with supporting the "typesVersions" redirection so that an upcoming change in the TypeScript compiler will not result in unresolvable build breaks.

This has more to do with ensuring that the next release of TypeScript does not result in major breaks for anyone still using Node v9, as module augmentations cannot address the issue this is meant to solve. An augmentation cannot address the "Duplicate identifier" error that would result from having conflicting interface IteratorResult<T> and type IteratorResult<T> definitions, which is why we are employing "typesVersions" here. Even if we were to remove the definitions for Node v9, the existing previously published definitions would still be available and would run into this issue. I discussed this with @RyanCavanaugh and we believe this is a fix we should still take despite the EOL status of Node v9.

SimonSchick · 2019-04-18T18:10:42Z

@rbuckton I think our comments were a little off-topic and we weren't arguing the actual changes made here but rather making a note that we should remove them to avoid future churn :)

rbuckton · 2019-04-18T18:13:26Z

Hopefully the changes we've made to the Node definitions for "typesVersions" are comprehensive enough to avoid collisions with ECMAScript standard library definitions for the foreseeable future.

Flarna · 2019-04-18T18:32:36Z

But why not v6 and v4 then? It's more likely that users are still on v6/v4 then on v9 as they were LTS versions.

rbuckton · 2019-04-29T20:56:28Z

@Flarna or others, can you take a look? I'd like to merge this if possible as changes continue to be made to the v6 and v8 definitions.

Flarna · 2019-04-29T21:40:34Z

types/node/v6/index.d.ts

-interface Error {
-    stack?: string;
-}
+// NOTE: Augmentations for TypeScript 3.1 and later should use individual files for overrides


actually 3.2 (also applicable to other versions)

Flarna · 2019-04-29T21:47:57Z

types/node/v6/tsconfig.json

@@ -2,7 +2,7 @@
    "compilerOptions": {
        "module": "commonjs",
        "lib": [
-            "es5"
+            "es6"


why this change? I can remember about some complains that node typings don't work with lib es5, usually because of missing forward declarations. if es6 is used here this is not detected by tests

Flarna · 2019-04-29T22:03:38Z

types/node/v6/index.d.ts

 interface IteratorResult<T> { }
 interface AsyncIterableIterator<T> {}
 interface SymbolConstructor {
    readonly iterator: symbol;
-    readonly asyncIterator: symbol;


have you removed asyncIterator per accident here? For whatever reason it's only present in v6 so maybe it's anyway wrong.

I removed it intentionally. It seemed out of place and I verified that it does not, in fact, exist in Node v6.

Flarna

I haven't looked on every single line but it seems correct to me

typescript-bot · 2019-04-29T22:26:56Z

A definition owner has approved this PR ⭐️. A maintainer will merge this PR shortly. If it shouldn't be merged yet, please leave a comment saying so and we'll wait. Thank you for your contribution to DefinitelyTyped!

typescript-bot · 2019-04-29T23:49:38Z

I just published @types/node@9.6.48 to npm.

typescript-bot · 2019-04-29T23:49:51Z

I just published @types/node@8.10.48 to npm.

typescript-bot · 2019-04-29T23:50:05Z

I just published @types/node@7.10.6 to npm.

typescript-bot · 2019-04-29T23:50:18Z

I just published @types/node@6.14.6 to npm.

sharikovvladislav · 2019-10-24T10:39:54Z

Isn't this needed in @types/node@10.x.x?

I am getting following at typescript@3.6.4 + @types/node@10.9.4:

node_modules/@types/node/index.d.ts:169:11 - error TS2300: Duplicate identifier 'IteratorResult'.

169 interface IteratorResult<T> { }
              ~~~~~~~~~~~~~~

  node_modules/typescript/lib/lib.es2015.iterable.d.ts:41:6
    41 type IteratorResult<T, TReturn = any> = IteratorYieldResult<T> | IteratorReturnResult<TReturn>;
            ~~~~~~~~~~~~~~
    'IteratorResult' was also declared here.
node_modules/typescript/lib/lib.es2015.iterable.d.ts:41:6 - error TS2300: Duplicate identifier 'IteratorResult'.

41 type IteratorResult<T, TReturn = any> = IteratorYieldResult<T> | IteratorReturnResult<TReturn>;
        ~~~~~~~~~~~~~~

Flarna · 2019-10-24T11:01:00Z

@sharikovvladislav Try latest node 10 type definitions.

sharikovvladislav · 2019-10-24T11:03:49Z

Latest for 10 is 10.14.22. Is it ok if I update 10.9.x to 10.14.22?

Thank you for the fast response.

Flarna · 2019-10-24T11:14:32Z

At least the split for typescript versions is included there.

sharikovvladislav · 2019-10-24T11:19:30Z

Thank you. That helped.

rbuckton force-pushed the splitNode8and9 branch from ed91dcc to 7dcd98a Compare April 12, 2019 01:12

typescript-bot added this to Needs Author Attention in Pull Request Status Board Apr 12, 2019

typescript-bot added New Definition This PR creates a new definition package. Popular package This PR affects a popular package (as counted by NPM download counts). The Travis CI build failed labels Apr 12, 2019

rbuckton force-pushed the splitNode8and9 branch 2 times, most recently from 3d08fb8 to f2fca64 Compare April 17, 2019 19:14

rbuckton force-pushed the splitNode8and9 branch 2 times, most recently from 27c165e to 8f24a46 Compare April 17, 2019 19:41

Split node/v8 and node/v9

78c7f3c

rbuckton force-pushed the splitNode8and9 branch from 8f24a46 to 78c7f3c Compare April 17, 2019 20:29

rbuckton marked this pull request as ready for review April 17, 2019 20:57

typescript-bot moved this from Needs Author Attention to Review in Pull Request Status Board Apr 17, 2019

typescript-bot added Awaiting reviewer feedback and removed The Travis CI build failed labels Apr 17, 2019

typescript-bot moved this from Needs Author Attention to Review in Pull Request Status Board Apr 22, 2019

typescript-bot removed the Has Merge Conflict This PR can't be merged because it has a merge conflict. The author needs to update it. label Apr 22, 2019

typescript-bot moved this from Review to Needs Author Attention in Pull Request Status Board Apr 26, 2019

typescript-bot added the Has Merge Conflict This PR can't be merged because it has a merge conflict. The author needs to update it. label Apr 26, 2019

Merge branch 'master' into splitNode8and9

7dc2416

typescript-bot moved this from Needs Author Attention to Review in Pull Request Status Board Apr 29, 2019

typescript-bot removed the Has Merge Conflict This PR can't be merged because it has a merge conflict. The author needs to update it. label Apr 29, 2019

Flarna reviewed Apr 29, 2019

View reviewed changes

Flarna approved these changes Apr 29, 2019

View reviewed changes

typescript-bot moved this from Review to Check and Merge in Pull Request Status Board Apr 29, 2019

typescript-bot added Owner Approved A listed owner of this package signed off on the pull request. Merge:Express and removed Awaiting reviewer feedback labels Apr 29, 2019

rbuckton merged commit b1d706c into master Apr 29, 2019

Pull Request Status Board automation moved this from Check and Merge to Done Apr 29, 2019

rbuckton deleted the splitNode8and9 branch April 29, 2019 23:57

typescript-bot removed this from Done in Pull Request Status Board Apr 30, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[NodeJS] Support 'typesVersions' in node/v6 through node/v9 #34678

[NodeJS] Support 'typesVersions' in node/v6 through node/v9 #34678

rbuckton commented Apr 12, 2019

typescript-bot commented Apr 12, 2019 •

edited

typescript-bot commented Apr 12, 2019 •

edited

rbuckton commented Apr 12, 2019

SimonSchick commented Apr 12, 2019

sandersn commented Apr 16, 2019

typescript-bot commented Apr 17, 2019

typescript-bot commented Apr 17, 2019

rbuckton commented Apr 17, 2019

rbuckton commented Apr 17, 2019

Flarna commented Apr 18, 2019

SimonSchick commented Apr 18, 2019

rbuckton commented Apr 18, 2019

SimonSchick commented Apr 18, 2019

rbuckton commented Apr 18, 2019

Flarna commented Apr 18, 2019

rbuckton commented Apr 29, 2019

Flarna Apr 29, 2019 •

edited

Flarna Apr 29, 2019

Flarna Apr 29, 2019

rbuckton Apr 29, 2019

Flarna left a comment

typescript-bot commented Apr 29, 2019

typescript-bot commented Apr 29, 2019

typescript-bot commented Apr 29, 2019

typescript-bot commented Apr 29, 2019

typescript-bot commented Apr 29, 2019

sharikovvladislav commented Oct 24, 2019 •

edited

Flarna commented Oct 24, 2019

sharikovvladislav commented Oct 24, 2019 •

edited

Flarna commented Oct 24, 2019

sharikovvladislav commented Oct 24, 2019

[NodeJS] Support 'typesVersions' in node/v6 through node/v9 #34678

[NodeJS] Support 'typesVersions' in node/v6 through node/v9 #34678

Conversation

rbuckton commented Apr 12, 2019

typescript-bot commented Apr 12, 2019 • edited

typescript-bot commented Apr 12, 2019 • edited

rbuckton commented Apr 12, 2019

SimonSchick commented Apr 12, 2019

sandersn commented Apr 16, 2019

typescript-bot commented Apr 17, 2019

typescript-bot commented Apr 17, 2019

rbuckton commented Apr 17, 2019

rbuckton commented Apr 17, 2019

Flarna commented Apr 18, 2019

SimonSchick commented Apr 18, 2019

rbuckton commented Apr 18, 2019

SimonSchick commented Apr 18, 2019

rbuckton commented Apr 18, 2019

Flarna commented Apr 18, 2019

rbuckton commented Apr 29, 2019

Flarna Apr 29, 2019 • edited

Choose a reason for hiding this comment

Flarna Apr 29, 2019

Choose a reason for hiding this comment

Flarna Apr 29, 2019

Choose a reason for hiding this comment

rbuckton Apr 29, 2019

Choose a reason for hiding this comment

Flarna left a comment

Choose a reason for hiding this comment

typescript-bot commented Apr 29, 2019

typescript-bot commented Apr 29, 2019

typescript-bot commented Apr 29, 2019

typescript-bot commented Apr 29, 2019

typescript-bot commented Apr 29, 2019

sharikovvladislav commented Oct 24, 2019 • edited

Flarna commented Oct 24, 2019

sharikovvladislav commented Oct 24, 2019 • edited

Flarna commented Oct 24, 2019

sharikovvladislav commented Oct 24, 2019

typescript-bot commented Apr 12, 2019 •

edited

typescript-bot commented Apr 12, 2019 •

edited

Flarna Apr 29, 2019 •

edited

sharikovvladislav commented Oct 24, 2019 •

edited

sharikovvladislav commented Oct 24, 2019 •

edited