[node] Added `globalThis` support to `global` by JasonHK · Pull Request #43308 · DefinitelyTyped/DefinitelyTyped

JasonHK · 2020-03-22T09:40:36Z

Please fill in this template.

Use a meaningful title for the pull request. Include the name of the package modified.
Test the change in your own code. (Compile and run.)
Add or edit tests to reflect the change. (Run with npm test.)
Follow the advice from the readme.
Avoid common mistakes.
Run npm run lint package-name (or tsc if no tslint.json is present).

Select one of these and delete the others:

If changing an existing definition:

Provide a URL to documentation or source code which provides context for the suggested changes: https://node.green/#ES2020-features-globalThis
If this PR brings the type definitions up to date with a new version of the JS library, update the version number in the header.
If you are making substantial changes, consider adding a tslint.json containing { "extends": "dtslint/dt.json" }. If for reason the any rule need to be disabled, disable it for that line using // tslint:disable-next-line [ruleName] and not for whole package so that the need for disabling can be reviewed.

orta · 2020-03-22T09:40:41Z

@JasonHK Thank you for submitting this PR!

This PR doesn't modify any tests, so it's hard to know what's being fixed, and your changes might regress in the future. Have you considered adding tests to cover the change you're making? Including tests allows this PR to be merged by yourself and the owners of this module.

Code Reviews

Because this is a widely-used package, a DT maintainer will need to review it before it can be merged.

🔔 @microsoft @DefinitelyTyped @jkomyno @a-tarasyuk @alvis @r3nya @btoueg @BrunoScheufler @smac89 @tellnes @Touffy @DeividasBakanas @eyqs @Flarna @Hannes-Magnusson-CK @KSXGitHub @hoo29 @kjin @ajafff @islishude @mwiktorczyk @mohsen1 @n-e @octo-sniffle @galkin @parambirs @eps1lon @SimonSchick @ThomasdenH @WilcoBakker @wwwy3y3 @ZaneHannanAU @samuela @kuehlein @j-oliveras @bhongy @chyzwar @trivikr @nguymin4 @yoursunny @qwelias @ExE-Boss @Ryan-Willpower @peterblazejewicz - please review this PR in the next few days. Be sure to explicitly select Approve or Request Changes in the GitHub UI so I know what's going on.

Status

✅ No merge conflicts
❌ Continuous integration tests have passed
❌ Only a DT maintainer can merge changes without tests

Once every item on this list is checked, I'll ask you for permission to merge and publish the changes.

Diagnostic Information: What the bot saw about this PR

{
  "type": "info",
  "pr_number": 43308,
  "author": "JasonHK",
  "owners": [
    "Microsoft",
    "DefinitelyTyped",
    "jkomyno",
    "a-tarasyuk",
    "alvis",
    "r3nya",
    "btoueg",
    "brunoscheufler",
    "smac89",
    "tellnes",
    "touffy",
    "DeividasBakanas",
    "eyqs",
    "Flarna",
    "Hannes-Magnusson-CK",
    "KSXGitHub",
    "hoo29",
    "kjin",
    "ajafff",
    "islishude",
    "mwiktorczyk",
    "mohsen1",
    "n-e",
    "octo-sniffle",
    "galkin",
    "parambirs",
    "eps1lon",
    "SimonSchick",
    "ThomasdenH",
    "WilcoBakker",
    "wwwy3y3",
    "ZaneHannanAU",
    "samuela",
    "kuehlein",
    "j-oliveras",
    "bhongy",
    "chyzwar",
    "trivikr",
    "nguymin4",
    "yoursunny",
    "qwelias",
    "ExE-Boss",
    "Ryan-Willpower",
    "peterblazejewicz"
  ],
  "dangerLevel": "ScopedAndUntested",
  "headCommitAbbrOid": "848f0e1",
  "headCommitOid": "848f0e1229eb62982db892e3880b1ec7d786123b",
  "mergeIsRequested": false,
  "stalenessInDays": 0,
  "lastCommitDate": "2020-03-22T09:38:36.000Z",
  "reviewLink": "https://github.com/DefinitelyTyped/DefinitelyTyped/pull/43308/files",
  "hasMergeConflict": false,
  "authorIsOwner": false,
  "isFirstContribution": false,
  "popularityLevel": "Critical",
  "anyPackageIsNew": false,
  "packages": [
    "node"
  ],
  "files": [
    {
      "filePath": "types/node/globals.d.ts",
      "kind": "definition",
      "package": "node"
    },
    {
      "filePath": "types/node/index.d.ts",
      "kind": "definition",
      "package": "node"
    },
    {
      "filePath": "types/node/v12/globals.d.ts",
      "kind": "definition",
      "package": "node"
    },
    {
      "filePath": "types/node/v12/index.d.ts",
      "kind": "definition",
      "package": "node"
    }
  ],
  "otherApprovalCount": 0,
  "ownerApprovalCount": 0,
  "maintainerApprovalCount": 0,
  "hasDismissedReview": false,
  "travisResult": "missing",
  "reviewersWithStaleReviews": [],
  "approvalFlags": 0,
  "isChangesRequested": false
}

orta · 2020-03-22T09:40:57Z

@JasonHK Thank you for submitting this PR!

This PR doesn't modify any tests, so it's hard to know what's being fixed, and your changes might regress in the future. Have you considered adding tests to cover the change you're making? Including tests allows this PR to be merged by yourself and the owners of this module.

Code Reviews

Because this is a widely-used package, a DT maintainer will need to review it before it can be merged.

🔔 @microsoft @DefinitelyTyped @jkomyno @a-tarasyuk @alvis @r3nya @btoueg @BrunoScheufler @smac89 @tellnes @Touffy @DeividasBakanas @eyqs @Flarna @Hannes-Magnusson-CK @KSXGitHub @hoo29 @kjin @ajafff @islishude @mwiktorczyk @mohsen1 @n-e @octo-sniffle @galkin @parambirs @eps1lon @SimonSchick @ThomasdenH @WilcoBakker @wwwy3y3 @ZaneHannanAU @samuela @kuehlein @j-oliveras @bhongy @chyzwar @trivikr @nguymin4 @yoursunny @qwelias @ExE-Boss @Ryan-Willpower @peterblazejewicz - please review this PR in the next few days. Be sure to explicitly select Approve or Request Changes in the GitHub UI so I know what's going on.

Status

✅ No merge conflicts
❌ Continuous integration tests have passed
❌ Only a DT maintainer can merge changes without tests

Once every item on this list is checked, I'll ask you for permission to merge and publish the changes.

Diagnostic Information: What the bot saw about this PR

{
  "type": "info",
  "pr_number": 43308,
  "author": "JasonHK",
  "owners": [
    "Microsoft",
    "DefinitelyTyped",
    "jkomyno",
    "a-tarasyuk",
    "alvis",
    "r3nya",
    "btoueg",
    "brunoscheufler",
    "smac89",
    "tellnes",
    "touffy",
    "DeividasBakanas",
    "eyqs",
    "Flarna",
    "Hannes-Magnusson-CK",
    "KSXGitHub",
    "hoo29",
    "kjin",
    "ajafff",
    "islishude",
    "mwiktorczyk",
    "mohsen1",
    "n-e",
    "octo-sniffle",
    "galkin",
    "parambirs",
    "eps1lon",
    "SimonSchick",
    "ThomasdenH",
    "WilcoBakker",
    "wwwy3y3",
    "ZaneHannanAU",
    "samuela",
    "kuehlein",
    "j-oliveras",
    "bhongy",
    "chyzwar",
    "trivikr",
    "nguymin4",
    "yoursunny",
    "qwelias",
    "ExE-Boss",
    "Ryan-Willpower",
    "peterblazejewicz"
  ],
  "dangerLevel": "ScopedAndUntested",
  "headCommitAbbrOid": "848f0e1",
  "headCommitOid": "848f0e1229eb62982db892e3880b1ec7d786123b",
  "mergeIsRequested": false,
  "stalenessInDays": 0,
  "lastCommitDate": "2020-03-22T09:38:36.000Z",
  "reviewLink": "https://github.com/DefinitelyTyped/DefinitelyTyped/pull/43308/files",
  "hasMergeConflict": false,
  "authorIsOwner": false,
  "isFirstContribution": false,
  "popularityLevel": "Critical",
  "anyPackageIsNew": false,
  "packages": [
    "node"
  ],
  "files": [
    {
      "filePath": "types/node/globals.d.ts",
      "kind": "definition",
      "package": "node"
    },
    {
      "filePath": "types/node/index.d.ts",
      "kind": "definition",
      "package": "node"
    },
    {
      "filePath": "types/node/v12/globals.d.ts",
      "kind": "definition",
      "package": "node"
    },
    {
      "filePath": "types/node/v12/index.d.ts",
      "kind": "definition",
      "package": "node"
    }
  ],
  "otherApprovalCount": 0,
  "ownerApprovalCount": 0,
  "maintainerApprovalCount": 0,
  "hasDismissedReview": false,
  "travisResult": "unknown",
  "reviewersWithStaleReviews": [],
  "approvalFlags": 0,
  "isChangesRequested": false
}

orta · 2020-03-22T09:40:59Z

@JasonHK Thank you for submitting this PR!

This PR doesn't modify any tests, so it's hard to know what's being fixed, and your changes might regress in the future. Have you considered adding tests to cover the change you're making? Including tests allows this PR to be merged by yourself and the owners of this module.

Code Reviews

Because this is a widely-used package, a DT maintainer will need to review it before it can be merged.

🔔 @microsoft @DefinitelyTyped @jkomyno @a-tarasyuk @alvis @r3nya @btoueg @BrunoScheufler @smac89 @tellnes @Touffy @DeividasBakanas @eyqs @Flarna @Hannes-Magnusson-CK @KSXGitHub @hoo29 @kjin @ajafff @islishude @mwiktorczyk @mohsen1 @n-e @octo-sniffle @galkin @parambirs @eps1lon @SimonSchick @ThomasdenH @WilcoBakker @wwwy3y3 @ZaneHannanAU @samuela @kuehlein @j-oliveras @bhongy @chyzwar @trivikr @nguymin4 @yoursunny @qwelias @ExE-Boss @Ryan-Willpower @peterblazejewicz - please review this PR in the next few days. Be sure to explicitly select Approve or Request Changes in the GitHub UI so I know what's going on.

Status

✅ No merge conflicts
❌ Continuous integration tests have passed
❌ Only a DT maintainer can merge changes without tests

Once every item on this list is checked, I'll ask you for permission to merge and publish the changes.

Diagnostic Information: What the bot saw about this PR

{
  "type": "info",
  "pr_number": 43308,
  "author": "JasonHK",
  "owners": [
    "Microsoft",
    "DefinitelyTyped",
    "jkomyno",
    "a-tarasyuk",
    "alvis",
    "r3nya",
    "btoueg",
    "brunoscheufler",
    "smac89",
    "tellnes",
    "touffy",
    "DeividasBakanas",
    "eyqs",
    "Flarna",
    "Hannes-Magnusson-CK",
    "KSXGitHub",
    "hoo29",
    "kjin",
    "ajafff",
    "islishude",
    "mwiktorczyk",
    "mohsen1",
    "n-e",
    "octo-sniffle",
    "galkin",
    "parambirs",
    "eps1lon",
    "SimonSchick",
    "ThomasdenH",
    "WilcoBakker",
    "wwwy3y3",
    "ZaneHannanAU",
    "samuela",
    "kuehlein",
    "j-oliveras",
    "bhongy",
    "chyzwar",
    "trivikr",
    "nguymin4",
    "yoursunny",
    "qwelias",
    "ExE-Boss",
    "Ryan-Willpower",
    "peterblazejewicz"
  ],
  "dangerLevel": "ScopedAndUntested",
  "headCommitAbbrOid": "848f0e1",
  "headCommitOid": "848f0e1229eb62982db892e3880b1ec7d786123b",
  "mergeIsRequested": false,
  "stalenessInDays": 0,
  "lastCommitDate": "2020-03-22T09:38:36.000Z",
  "reviewLink": "https://github.com/DefinitelyTyped/DefinitelyTyped/pull/43308/files",
  "hasMergeConflict": false,
  "authorIsOwner": false,
  "isFirstContribution": false,
  "popularityLevel": "Critical",
  "anyPackageIsNew": false,
  "packages": [
    "node"
  ],
  "files": [
    {
      "filePath": "types/node/globals.d.ts",
      "kind": "definition",
      "package": "node"
    },
    {
      "filePath": "types/node/index.d.ts",
      "kind": "definition",
      "package": "node"
    },
    {
      "filePath": "types/node/v12/globals.d.ts",
      "kind": "definition",
      "package": "node"
    },
    {
      "filePath": "types/node/v12/index.d.ts",
      "kind": "definition",
      "package": "node"
    }
  ],
  "otherApprovalCount": 0,
  "ownerApprovalCount": 0,
  "maintainerApprovalCount": 0,
  "hasDismissedReview": false,
  "travisResult": "unknown",
  "reviewersWithStaleReviews": [],
  "approvalFlags": 0,
  "isChangesRequested": false
}

orta · 2020-03-22T09:41:01Z

@JasonHK Thank you for submitting this PR!

This PR doesn't modify any tests, so it's hard to know what's being fixed, and your changes might regress in the future. Have you considered adding tests to cover the change you're making? Including tests allows this PR to be merged by yourself and the owners of this module.

Code Reviews

Because this is a widely-used package, a DT maintainer will need to review it before it can be merged.

🔔 @microsoft @DefinitelyTyped @jkomyno @a-tarasyuk @alvis @r3nya @btoueg @BrunoScheufler @smac89 @tellnes @Touffy @DeividasBakanas @eyqs @Flarna @Hannes-Magnusson-CK @KSXGitHub @hoo29 @kjin @ajafff @islishude @mwiktorczyk @mohsen1 @n-e @octo-sniffle @galkin @parambirs @eps1lon @SimonSchick @ThomasdenH @WilcoBakker @wwwy3y3 @ZaneHannanAU @samuela @kuehlein @j-oliveras @bhongy @chyzwar @trivikr @nguymin4 @yoursunny @qwelias @ExE-Boss @Ryan-Willpower @peterblazejewicz - please review this PR in the next few days. Be sure to explicitly select Approve or Request Changes in the GitHub UI so I know what's going on.

Status

✅ No merge conflicts
❌ Continuous integration tests have passed
❌ Only a DT maintainer can merge changes without tests

Once every item on this list is checked, I'll ask you for permission to merge and publish the changes.

Diagnostic Information: What the bot saw about this PR

{
  "type": "info",
  "pr_number": 43308,
  "author": "JasonHK",
  "owners": [
    "Microsoft",
    "DefinitelyTyped",
    "jkomyno",
    "a-tarasyuk",
    "alvis",
    "r3nya",
    "btoueg",
    "brunoscheufler",
    "smac89",
    "tellnes",
    "touffy",
    "DeividasBakanas",
    "eyqs",
    "Flarna",
    "Hannes-Magnusson-CK",
    "KSXGitHub",
    "hoo29",
    "kjin",
    "ajafff",
    "islishude",
    "mwiktorczyk",
    "mohsen1",
    "n-e",
    "octo-sniffle",
    "galkin",
    "parambirs",
    "eps1lon",
    "SimonSchick",
    "ThomasdenH",
    "WilcoBakker",
    "wwwy3y3",
    "ZaneHannanAU",
    "samuela",
    "kuehlein",
    "j-oliveras",
    "bhongy",
    "chyzwar",
    "trivikr",
    "nguymin4",
    "yoursunny",
    "qwelias",
    "ExE-Boss",
    "Ryan-Willpower",
    "peterblazejewicz"
  ],
  "dangerLevel": "ScopedAndUntested",
  "headCommitAbbrOid": "848f0e1",
  "headCommitOid": "848f0e1229eb62982db892e3880b1ec7d786123b",
  "mergeIsRequested": false,
  "stalenessInDays": 0,
  "lastCommitDate": "2020-03-22T09:38:36.000Z",
  "reviewLink": "https://github.com/DefinitelyTyped/DefinitelyTyped/pull/43308/files",
  "hasMergeConflict": false,
  "authorIsOwner": false,
  "isFirstContribution": false,
  "popularityLevel": "Critical",
  "anyPackageIsNew": false,
  "packages": [
    "node"
  ],
  "files": [
    {
      "filePath": "types/node/globals.d.ts",
      "kind": "definition",
      "package": "node"
    },
    {
      "filePath": "types/node/index.d.ts",
      "kind": "definition",
      "package": "node"
    },
    {
      "filePath": "types/node/v12/globals.d.ts",
      "kind": "definition",
      "package": "node"
    },
    {
      "filePath": "types/node/v12/index.d.ts",
      "kind": "definition",
      "package": "node"
    }
  ],
  "otherApprovalCount": 0,
  "ownerApprovalCount": 0,
  "maintainerApprovalCount": 0,
  "hasDismissedReview": false,
  "travisResult": "unknown",
  "reviewersWithStaleReviews": [],
  "approvalFlags": 0,
  "isChangesRequested": false
}

orta · 2020-03-22T09:41:02Z

@JasonHK Thank you for submitting this PR!

This PR doesn't modify any tests, so it's hard to know what's being fixed, and your changes might regress in the future. Have you considered adding tests to cover the change you're making? Including tests allows this PR to be merged by yourself and the owners of this module.

Code Reviews

Because this is a widely-used package, a DT maintainer will need to review it before it can be merged.

🔔 @microsoft @DefinitelyTyped @jkomyno @a-tarasyuk @alvis @r3nya @btoueg @BrunoScheufler @smac89 @tellnes @Touffy @DeividasBakanas @eyqs @Flarna @Hannes-Magnusson-CK @KSXGitHub @hoo29 @kjin @ajafff @islishude @mwiktorczyk @mohsen1 @n-e @octo-sniffle @galkin @parambirs @eps1lon @SimonSchick @ThomasdenH @WilcoBakker @wwwy3y3 @ZaneHannanAU @samuela @kuehlein @j-oliveras @bhongy @chyzwar @trivikr @nguymin4 @yoursunny @qwelias @ExE-Boss @Ryan-Willpower @peterblazejewicz - please review this PR in the next few days. Be sure to explicitly select Approve or Request Changes in the GitHub UI so I know what's going on.

Status

✅ No merge conflicts
❌ Continuous integration tests have passed
❌ Only a DT maintainer can merge changes without tests

Once every item on this list is checked, I'll ask you for permission to merge and publish the changes.

Diagnostic Information: What the bot saw about this PR

{
  "type": "info",
  "pr_number": 43308,
  "author": "JasonHK",
  "owners": [
    "Microsoft",
    "DefinitelyTyped",
    "jkomyno",
    "a-tarasyuk",
    "alvis",
    "r3nya",
    "btoueg",
    "brunoscheufler",
    "smac89",
    "tellnes",
    "touffy",
    "DeividasBakanas",
    "eyqs",
    "Flarna",
    "Hannes-Magnusson-CK",
    "KSXGitHub",
    "hoo29",
    "kjin",
    "ajafff",
    "islishude",
    "mwiktorczyk",
    "mohsen1",
    "n-e",
    "octo-sniffle",
    "galkin",
    "parambirs",
    "eps1lon",
    "SimonSchick",
    "ThomasdenH",
    "WilcoBakker",
    "wwwy3y3",
    "ZaneHannanAU",
    "samuela",
    "kuehlein",
    "j-oliveras",
    "bhongy",
    "chyzwar",
    "trivikr",
    "nguymin4",
    "yoursunny",
    "qwelias",
    "ExE-Boss",
    "Ryan-Willpower",
    "peterblazejewicz"
  ],
  "dangerLevel": "ScopedAndUntested",
  "headCommitAbbrOid": "848f0e1",
  "headCommitOid": "848f0e1229eb62982db892e3880b1ec7d786123b",
  "mergeIsRequested": false,
  "stalenessInDays": 0,
  "lastCommitDate": "2020-03-22T09:38:36.000Z",
  "reviewLink": "https://github.com/DefinitelyTyped/DefinitelyTyped/pull/43308/files",
  "hasMergeConflict": false,
  "authorIsOwner": false,
  "isFirstContribution": false,
  "popularityLevel": "Critical",
  "anyPackageIsNew": false,
  "packages": [
    "node"
  ],
  "files": [
    {
      "filePath": "types/node/globals.d.ts",
      "kind": "definition",
      "package": "node"
    },
    {
      "filePath": "types/node/index.d.ts",
      "kind": "definition",
      "package": "node"
    },
    {
      "filePath": "types/node/v12/globals.d.ts",
      "kind": "definition",
      "package": "node"
    },
    {
      "filePath": "types/node/v12/index.d.ts",
      "kind": "definition",
      "package": "node"
    }
  ],
  "otherApprovalCount": 0,
  "ownerApprovalCount": 0,
  "maintainerApprovalCount": 0,
  "hasDismissedReview": false,
  "travisResult": "unknown",
  "reviewersWithStaleReviews": [],
  "approvalFlags": 0,
  "isChangesRequested": false
}

orta · 2020-03-22T09:41:04Z

@JasonHK Thank you for submitting this PR!

This PR doesn't modify any tests, so it's hard to know what's being fixed, and your changes might regress in the future. Have you considered adding tests to cover the change you're making? Including tests allows this PR to be merged by yourself and the owners of this module.

Code Reviews

Because this is a widely-used package, a DT maintainer will need to review it before it can be merged.

🔔 @microsoft @DefinitelyTyped @jkomyno @a-tarasyuk @alvis @r3nya @btoueg @BrunoScheufler @smac89 @tellnes @Touffy @DeividasBakanas @eyqs @Flarna @Hannes-Magnusson-CK @KSXGitHub @hoo29 @kjin @ajafff @islishude @mwiktorczyk @mohsen1 @n-e @octo-sniffle @galkin @parambirs @eps1lon @SimonSchick @ThomasdenH @WilcoBakker @wwwy3y3 @ZaneHannanAU @samuela @kuehlein @j-oliveras @bhongy @chyzwar @trivikr @nguymin4 @yoursunny @qwelias @ExE-Boss @Ryan-Willpower @peterblazejewicz - please review this PR in the next few days. Be sure to explicitly select Approve or Request Changes in the GitHub UI so I know what's going on.

Status

✅ No merge conflicts
❌ Continuous integration tests have passed
❌ Only a DT maintainer can merge changes without tests

Once every item on this list is checked, I'll ask you for permission to merge and publish the changes.

Diagnostic Information: What the bot saw about this PR

{
  "type": "info",
  "pr_number": 43308,
  "author": "JasonHK",
  "owners": [
    "Microsoft",
    "DefinitelyTyped",
    "jkomyno",
    "a-tarasyuk",
    "alvis",
    "r3nya",
    "btoueg",
    "brunoscheufler",
    "smac89",
    "tellnes",
    "touffy",
    "DeividasBakanas",
    "eyqs",
    "Flarna",
    "Hannes-Magnusson-CK",
    "KSXGitHub",
    "hoo29",
    "kjin",
    "ajafff",
    "islishude",
    "mwiktorczyk",
    "mohsen1",
    "n-e",
    "octo-sniffle",
    "galkin",
    "parambirs",
    "eps1lon",
    "SimonSchick",
    "ThomasdenH",
    "WilcoBakker",
    "wwwy3y3",
    "ZaneHannanAU",
    "samuela",
    "kuehlein",
    "j-oliveras",
    "bhongy",
    "chyzwar",
    "trivikr",
    "nguymin4",
    "yoursunny",
    "qwelias",
    "ExE-Boss",
    "Ryan-Willpower",
    "peterblazejewicz"
  ],
  "dangerLevel": "ScopedAndUntested",
  "headCommitAbbrOid": "848f0e1",
  "headCommitOid": "848f0e1229eb62982db892e3880b1ec7d786123b",
  "mergeIsRequested": false,
  "stalenessInDays": 0,
  "lastCommitDate": "2020-03-22T09:38:36.000Z",
  "reviewLink": "https://github.com/DefinitelyTyped/DefinitelyTyped/pull/43308/files",
  "hasMergeConflict": false,
  "authorIsOwner": false,
  "isFirstContribution": false,
  "popularityLevel": "Critical",
  "anyPackageIsNew": false,
  "packages": [
    "node"
  ],
  "files": [
    {
      "filePath": "types/node/globals.d.ts",
      "kind": "definition",
      "package": "node"
    },
    {
      "filePath": "types/node/index.d.ts",
      "kind": "definition",
      "package": "node"
    },
    {
      "filePath": "types/node/v12/globals.d.ts",
      "kind": "definition",
      "package": "node"
    },
    {
      "filePath": "types/node/v12/index.d.ts",
      "kind": "definition",
      "package": "node"
    }
  ],
  "otherApprovalCount": 0,
  "ownerApprovalCount": 0,
  "maintainerApprovalCount": 0,
  "hasDismissedReview": false,
  "travisResult": "unknown",
  "reviewersWithStaleReviews": [],
  "approvalFlags": 0,
  "isChangesRequested": false
}

orta · 2020-03-22T09:41:04Z

@JasonHK Thank you for submitting this PR!

This PR doesn't modify any tests, so it's hard to know what's being fixed, and your changes might regress in the future. Have you considered adding tests to cover the change you're making? Including tests allows this PR to be merged by yourself and the owners of this module.

Code Reviews

Because this is a widely-used package, a DT maintainer will need to review it before it can be merged.

🔔 @microsoft @DefinitelyTyped @jkomyno @a-tarasyuk @alvis @r3nya @btoueg @BrunoScheufler @smac89 @tellnes @Touffy @DeividasBakanas @eyqs @Flarna @Hannes-Magnusson-CK @KSXGitHub @hoo29 @kjin @ajafff @islishude @mwiktorczyk @mohsen1 @n-e @octo-sniffle @galkin @parambirs @eps1lon @SimonSchick @ThomasdenH @WilcoBakker @wwwy3y3 @ZaneHannanAU @samuela @kuehlein @j-oliveras @bhongy @chyzwar @trivikr @nguymin4 @yoursunny @qwelias @ExE-Boss @Ryan-Willpower @peterblazejewicz - please review this PR in the next few days. Be sure to explicitly select Approve or Request Changes in the GitHub UI so I know what's going on.

Status

✅ No merge conflicts
❌ Continuous integration tests have passed
❌ Only a DT maintainer can merge changes without tests

Once every item on this list is checked, I'll ask you for permission to merge and publish the changes.

Diagnostic Information: What the bot saw about this PR

{
  "type": "info",
  "pr_number": 43308,
  "author": "JasonHK",
  "owners": [
    "Microsoft",
    "DefinitelyTyped",
    "jkomyno",
    "a-tarasyuk",
    "alvis",
    "r3nya",
    "btoueg",
    "brunoscheufler",
    "smac89",
    "tellnes",
    "touffy",
    "DeividasBakanas",
    "eyqs",
    "Flarna",
    "Hannes-Magnusson-CK",
    "KSXGitHub",
    "hoo29",
    "kjin",
    "ajafff",
    "islishude",
    "mwiktorczyk",
    "mohsen1",
    "n-e",
    "octo-sniffle",
    "galkin",
    "parambirs",
    "eps1lon",
    "SimonSchick",
    "ThomasdenH",
    "WilcoBakker",
    "wwwy3y3",
    "ZaneHannanAU",
    "samuela",
    "kuehlein",
    "j-oliveras",
    "bhongy",
    "chyzwar",
    "trivikr",
    "nguymin4",
    "yoursunny",
    "qwelias",
    "ExE-Boss",
    "Ryan-Willpower",
    "peterblazejewicz"
  ],
  "dangerLevel": "ScopedAndUntested",
  "headCommitAbbrOid": "848f0e1",
  "headCommitOid": "848f0e1229eb62982db892e3880b1ec7d786123b",
  "mergeIsRequested": false,
  "stalenessInDays": 0,
  "lastCommitDate": "2020-03-22T09:38:36.000Z",
  "reviewLink": "https://github.com/DefinitelyTyped/DefinitelyTyped/pull/43308/files",
  "hasMergeConflict": false,
  "authorIsOwner": false,
  "isFirstContribution": false,
  "popularityLevel": "Critical",
  "anyPackageIsNew": false,
  "packages": [
    "node"
  ],
  "files": [
    {
      "filePath": "types/node/globals.d.ts",
      "kind": "definition",
      "package": "node"
    },
    {
      "filePath": "types/node/index.d.ts",
      "kind": "definition",
      "package": "node"
    },
    {
      "filePath": "types/node/v12/globals.d.ts",
      "kind": "definition",
      "package": "node"
    },
    {
      "filePath": "types/node/v12/index.d.ts",
      "kind": "definition",
      "package": "node"
    }
  ],
  "otherApprovalCount": 0,
  "ownerApprovalCount": 0,
  "maintainerApprovalCount": 0,
  "hasDismissedReview": false,
  "travisResult": "unknown",
  "reviewersWithStaleReviews": [],
  "approvalFlags": 0,
  "isChangesRequested": false
}

orta · 2020-03-22T09:41:04Z

@JasonHK Thank you for submitting this PR!

This PR doesn't modify any tests, so it's hard to know what's being fixed, and your changes might regress in the future. Have you considered adding tests to cover the change you're making? Including tests allows this PR to be merged by yourself and the owners of this module.

Code Reviews

Because this is a widely-used package, a DT maintainer will need to review it before it can be merged.

🔔 @microsoft @DefinitelyTyped @jkomyno @a-tarasyuk @alvis @r3nya @btoueg @BrunoScheufler @smac89 @tellnes @Touffy @DeividasBakanas @eyqs @Flarna @Hannes-Magnusson-CK @KSXGitHub @hoo29 @kjin @ajafff @islishude @mwiktorczyk @mohsen1 @n-e @octo-sniffle @galkin @parambirs @eps1lon @SimonSchick @ThomasdenH @WilcoBakker @wwwy3y3 @ZaneHannanAU @samuela @kuehlein @j-oliveras @bhongy @chyzwar @trivikr @nguymin4 @yoursunny @qwelias @ExE-Boss @Ryan-Willpower @peterblazejewicz - please review this PR in the next few days. Be sure to explicitly select Approve or Request Changes in the GitHub UI so I know what's going on.

Status

✅ No merge conflicts
❌ Continuous integration tests have passed
❌ Only a DT maintainer can merge changes without tests

Once every item on this list is checked, I'll ask you for permission to merge and publish the changes.

Diagnostic Information: What the bot saw about this PR

{
  "type": "info",
  "pr_number": 43308,
  "author": "JasonHK",
  "owners": [
    "Microsoft",
    "DefinitelyTyped",
    "jkomyno",
    "a-tarasyuk",
    "alvis",
    "r3nya",
    "btoueg",
    "brunoscheufler",
    "smac89",
    "tellnes",
    "touffy",
    "DeividasBakanas",
    "eyqs",
    "Flarna",
    "Hannes-Magnusson-CK",
    "KSXGitHub",
    "hoo29",
    "kjin",
    "ajafff",
    "islishude",
    "mwiktorczyk",
    "mohsen1",
    "n-e",
    "octo-sniffle",
    "galkin",
    "parambirs",
    "eps1lon",
    "SimonSchick",
    "ThomasdenH",
    "WilcoBakker",
    "wwwy3y3",
    "ZaneHannanAU",
    "samuela",
    "kuehlein",
    "j-oliveras",
    "bhongy",
    "chyzwar",
    "trivikr",
    "nguymin4",
    "yoursunny",
    "qwelias",
    "ExE-Boss",
    "Ryan-Willpower",
    "peterblazejewicz"
  ],
  "dangerLevel": "ScopedAndUntested",
  "headCommitAbbrOid": "848f0e1",
  "headCommitOid": "848f0e1229eb62982db892e3880b1ec7d786123b",
  "mergeIsRequested": false,
  "stalenessInDays": 0,
  "lastCommitDate": "2020-03-22T09:38:36.000Z",
  "reviewLink": "https://github.com/DefinitelyTyped/DefinitelyTyped/pull/43308/files",
  "hasMergeConflict": false,
  "authorIsOwner": false,
  "isFirstContribution": false,
  "popularityLevel": "Critical",
  "anyPackageIsNew": false,
  "packages": [
    "node"
  ],
  "files": [
    {
      "filePath": "types/node/globals.d.ts",
      "kind": "definition",
      "package": "node"
    },
    {
      "filePath": "types/node/index.d.ts",
      "kind": "definition",
      "package": "node"
    },
    {
      "filePath": "types/node/v12/globals.d.ts",
      "kind": "definition",
      "package": "node"
    },
    {
      "filePath": "types/node/v12/index.d.ts",
      "kind": "definition",
      "package": "node"
    }
  ],
  "otherApprovalCount": 0,
  "ownerApprovalCount": 0,
  "maintainerApprovalCount": 0,
  "hasDismissedReview": false,
  "travisResult": "unknown",
  "reviewersWithStaleReviews": [],
  "approvalFlags": 0,
  "isChangesRequested": false
}

orta · 2020-03-22T09:41:05Z

@JasonHK Thank you for submitting this PR!

This PR doesn't modify any tests, so it's hard to know what's being fixed, and your changes might regress in the future. Have you considered adding tests to cover the change you're making? Including tests allows this PR to be merged by yourself and the owners of this module.

Code Reviews

Because this is a widely-used package, a DT maintainer will need to review it before it can be merged.

🔔 @microsoft @DefinitelyTyped @jkomyno @a-tarasyuk @alvis @r3nya @btoueg @BrunoScheufler @smac89 @tellnes @Touffy @DeividasBakanas @eyqs @Flarna @Hannes-Magnusson-CK @KSXGitHub @hoo29 @kjin @ajafff @islishude @mwiktorczyk @mohsen1 @n-e @octo-sniffle @galkin @parambirs @eps1lon @SimonSchick @ThomasdenH @WilcoBakker @wwwy3y3 @ZaneHannanAU @samuela @kuehlein @j-oliveras @bhongy @chyzwar @trivikr @nguymin4 @yoursunny @qwelias @ExE-Boss @Ryan-Willpower @peterblazejewicz - please review this PR in the next few days. Be sure to explicitly select Approve or Request Changes in the GitHub UI so I know what's going on.

Status

✅ No merge conflicts
❌ Continuous integration tests have passed
❌ Only a DT maintainer can merge changes without tests

Once every item on this list is checked, I'll ask you for permission to merge and publish the changes.

Diagnostic Information: What the bot saw about this PR

{
  "type": "info",
  "pr_number": 43308,
  "author": "JasonHK",
  "owners": [
    "Microsoft",
    "DefinitelyTyped",
    "jkomyno",
    "a-tarasyuk",
    "alvis",
    "r3nya",
    "btoueg",
    "brunoscheufler",
    "smac89",
    "tellnes",
    "touffy",
    "DeividasBakanas",
    "eyqs",
    "Flarna",
    "Hannes-Magnusson-CK",
    "KSXGitHub",
    "hoo29",
    "kjin",
    "ajafff",
    "islishude",
    "mwiktorczyk",
    "mohsen1",
    "n-e",
    "octo-sniffle",
    "galkin",
    "parambirs",
    "eps1lon",
    "SimonSchick",
    "ThomasdenH",
    "WilcoBakker",
    "wwwy3y3",
    "ZaneHannanAU",
    "samuela",
    "kuehlein",
    "j-oliveras",
    "bhongy",
    "chyzwar",
    "trivikr",
    "nguymin4",
    "yoursunny",
    "qwelias",
    "ExE-Boss",
    "Ryan-Willpower",
    "peterblazejewicz"
  ],
  "dangerLevel": "ScopedAndUntested",
  "headCommitAbbrOid": "848f0e1",
  "headCommitOid": "848f0e1229eb62982db892e3880b1ec7d786123b",
  "mergeIsRequested": false,
  "stalenessInDays": 0,
  "lastCommitDate": "2020-03-22T09:38:36.000Z",
  "reviewLink": "https://github.com/DefinitelyTyped/DefinitelyTyped/pull/43308/files",
  "hasMergeConflict": false,
  "authorIsOwner": false,
  "isFirstContribution": false,
  "popularityLevel": "Critical",
  "anyPackageIsNew": false,
  "packages": [
    "node"
  ],
  "files": [
    {
      "filePath": "types/node/globals.d.ts",
      "kind": "definition",
      "package": "node"
    },
    {
      "filePath": "types/node/index.d.ts",
      "kind": "definition",
      "package": "node"
    },
    {
      "filePath": "types/node/v12/globals.d.ts",
      "kind": "definition",
      "package": "node"
    },
    {
      "filePath": "types/node/v12/index.d.ts",
      "kind": "definition",
      "package": "node"
    }
  ],
  "otherApprovalCount": 0,
  "ownerApprovalCount": 0,
  "maintainerApprovalCount": 0,
  "hasDismissedReview": false,
  "travisResult": "unknown",
  "reviewersWithStaleReviews": [],
  "approvalFlags": 0,
  "isChangesRequested": false
}

orta · 2020-03-22T09:41:07Z

@JasonHK Thank you for submitting this PR!

This PR doesn't modify any tests, so it's hard to know what's being fixed, and your changes might regress in the future. Have you considered adding tests to cover the change you're making? Including tests allows this PR to be merged by yourself and the owners of this module.

Code Reviews

Because this is a widely-used package, a DT maintainer will need to review it before it can be merged.

🔔 @microsoft @DefinitelyTyped @jkomyno @a-tarasyuk @alvis @r3nya @btoueg @BrunoScheufler @smac89 @tellnes @Touffy @DeividasBakanas @eyqs @Flarna @Hannes-Magnusson-CK @KSXGitHub @hoo29 @kjin @ajafff @islishude @mwiktorczyk @mohsen1 @n-e @octo-sniffle @galkin @parambirs @eps1lon @SimonSchick @ThomasdenH @WilcoBakker @wwwy3y3 @ZaneHannanAU @samuela @kuehlein @j-oliveras @bhongy @chyzwar @trivikr @nguymin4 @yoursunny @qwelias @ExE-Boss @Ryan-Willpower @peterblazejewicz - please review this PR in the next few days. Be sure to explicitly select Approve or Request Changes in the GitHub UI so I know what's going on.

Status

✅ No merge conflicts
❌ Continuous integration tests have passed
❌ Only a DT maintainer can merge changes without tests

Once every item on this list is checked, I'll ask you for permission to merge and publish the changes.

Diagnostic Information: What the bot saw about this PR

{
  "type": "info",
  "pr_number": 43308,
  "author": "JasonHK",
  "owners": [
    "Microsoft",
    "DefinitelyTyped",
    "jkomyno",
    "a-tarasyuk",
    "alvis",
    "r3nya",
    "btoueg",
    "brunoscheufler",
    "smac89",
    "tellnes",
    "touffy",
    "DeividasBakanas",
    "eyqs",
    "Flarna",
    "Hannes-Magnusson-CK",
    "KSXGitHub",
    "hoo29",
    "kjin",
    "ajafff",
    "islishude",
    "mwiktorczyk",
    "mohsen1",
    "n-e",
    "octo-sniffle",
    "galkin",
    "parambirs",
    "eps1lon",
    "SimonSchick",
    "ThomasdenH",
    "WilcoBakker",
    "wwwy3y3",
    "ZaneHannanAU",
    "samuela",
    "kuehlein",
    "j-oliveras",
    "bhongy",
    "chyzwar",
    "trivikr",
    "nguymin4",
    "yoursunny",
    "qwelias",
    "ExE-Boss",
    "Ryan-Willpower",
    "peterblazejewicz"
  ],
  "dangerLevel": "ScopedAndUntested",
  "headCommitAbbrOid": "848f0e1",
  "headCommitOid": "848f0e1229eb62982db892e3880b1ec7d786123b",
  "mergeIsRequested": false,
  "stalenessInDays": 0,
  "lastCommitDate": "2020-03-22T09:38:36.000Z",
  "reviewLink": "https://github.com/DefinitelyTyped/DefinitelyTyped/pull/43308/files",
  "hasMergeConflict": false,
  "authorIsOwner": false,
  "isFirstContribution": false,
  "popularityLevel": "Critical",
  "anyPackageIsNew": false,
  "packages": [
    "node"
  ],
  "files": [
    {
      "filePath": "types/node/globals.d.ts",
      "kind": "definition",
      "package": "node"
    },
    {
      "filePath": "types/node/index.d.ts",
      "kind": "definition",
      "package": "node"
    },
    {
      "filePath": "types/node/v12/globals.d.ts",
      "kind": "definition",
      "package": "node"
    },
    {
      "filePath": "types/node/v12/index.d.ts",
      "kind": "definition",
      "package": "node"
    }
  ],
  "otherApprovalCount": 0,
  "ownerApprovalCount": 0,
  "maintainerApprovalCount": 0,
  "hasDismissedReview": false,
  "travisResult": "unknown",
  "reviewersWithStaleReviews": [],
  "approvalFlags": 0,
  "isChangesRequested": false
}

typescript-bot · 2020-03-22T17:22:50Z

@JasonHK 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 · 2020-03-22T17:22:50Z

@JasonHK 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 · 2020-03-22T17:22:51Z

@JasonHK 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 · 2020-03-22T17:22:51Z

@JasonHK 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!

SimonSchick · 2020-03-22T18:09:33Z

Is adding globalThis support really worth this degree of fragmentation for node versions?

JasonHK · 2020-03-22T20:39:00Z

Is adding globalThis support really worth this degree of fragmentation for node versions?

@SimonSchick If we don't add globalThis support now, we'll still need to add this in the future anyway.

galkin

We need motivation for adding different typescript version support announced at this pull

galkin · 2020-03-22T20:42:43Z

types/node/index.d.ts

 // NOTE: TypeScript version-specific augmentations can be found in the following paths:
 //          - ~/base.d.ts         - Shared definitions common to all TypeScript versions
 //          - ~/index.d.ts        - Definitions specific to TypeScript 2.8
+//          - ~/ts3.2/index.d.ts  - Definitions specific to TypeScript 3.2


this lines looks dangerous for me. There are many options for node.js version, what we support.
This changes also add complexity for typescript version. As a result we have a matrix, what is too bit.

@galkin Actually, "~/ts3.2" already exists before this pull request, it's just missing from that comment.

Also, the approach I used here is the result of TypeScript error TS2403, this error makes me unable to simply override global.

JasonHK · 2020-03-23T08:44:43Z

We need motivation for adding different typescript version support announced at this pull

@galkin I think adding globalThis support to global is definitely worth it.

Before this, when declaring global variables to the global scope via declare global {...}, these variables will be available in the global scope, also in self and window, but not global.

declare global
{
    var __GLOBAL__: typeof globalThis;
}

__GLOBAL__;        // Working
self.__GLOBAL__;   // Working
window.__GLOBAL__; // Working
global.__GLOBAL__; // error TS2339: Property '__GLOBAL__' does not exist on type 'global'.

After this, that issue will be fixed.

__GLOBAL__;        // Working
self.__GLOBAL__;   // Working
window.__GLOBAL__; // Working
global.__GLOBAL__; // Working

typescript-bot · 2020-03-29T08:26:17Z

@JasonHK I haven't seen anything from you in a while and this PR currently has problems that prevent it from being merged. The PR will be closed tomorrow if there aren't new commits to fix the issues.

typescript-bot · 2020-03-30T08:27:05Z

@JasonHK To keep things tidy, we have to close PRs that aren't mergeable but don't have activity from their author. No worries, though - please open a new PR if you'd like to continue with this change. Thank you!

JasonHK added 2 commits March 22, 2020 17:28

[node] Added globalThis support to global

468b92e

[node] Updated "Definitions by"

848f0e1

orta added Where is Travis? Critical package and removed Where is Travis? labels Mar 22, 2020

orta added the The Travis CI build failed label Mar 22, 2020

orta added Where is Travis? and removed The Travis CI build failed labels Mar 22, 2020

orta added The Travis CI build failed and removed Where is Travis? labels Mar 22, 2020

orta added Where is Travis? and removed The Travis CI build failed labels Mar 22, 2020

typescript-bot added Where is Travis? New Definition This PR creates a new definition package. Popular package This PR affects a popular package (as counted by NPM download counts). and removed Where is Travis? labels Mar 22, 2020

typescript-bot removed New Definition This PR creates a new definition package. Popular package This PR affects a popular package (as counted by NPM download counts). labels Mar 22, 2020

galkin suggested changes Mar 22, 2020

View reviewed changes

[node] Updated tests

74aae7b

peterblazejewicz mentioned this pull request Apr 17, 2020

[node] Added globalThis support to global #43821

Closed

9 tasks

JasonHK mentioned this pull request May 13, 2020

[node] Added globalThis support to global #44700

Merged

10 tasks

Conversation

JasonHK commented Mar 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

orta commented Mar 22, 2020

Code Reviews

Status

Uh oh!

orta commented Mar 22, 2020

Code Reviews

Status

Uh oh!

orta commented Mar 22, 2020

Code Reviews

Status

Uh oh!

orta commented Mar 22, 2020

Code Reviews

Status

Uh oh!

orta commented Mar 22, 2020

Code Reviews

Status

Uh oh!

orta commented Mar 22, 2020

Code Reviews

Status

Uh oh!

orta commented Mar 22, 2020

Code Reviews

Status

Uh oh!

orta commented Mar 22, 2020

Code Reviews

Status

Uh oh!

orta commented Mar 22, 2020

Code Reviews

Status

Uh oh!

orta commented Mar 22, 2020

Code Reviews

Status

Uh oh!

typescript-bot commented Mar 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

typescript-bot commented Mar 22, 2020

Uh oh!

typescript-bot commented Mar 22, 2020

Uh oh!

typescript-bot commented Mar 22, 2020

Uh oh!

SimonSchick commented Mar 22, 2020

Uh oh!

JasonHK commented Mar 22, 2020

Uh oh!

galkin left a comment

Choose a reason for hiding this comment

Uh oh!

galkin Mar 22, 2020

Choose a reason for hiding this comment

Uh oh!

JasonHK Mar 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JasonHK commented Mar 23, 2020

Uh oh!

typescript-bot commented Mar 29, 2020

Uh oh!

typescript-bot commented Mar 30, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

JasonHK commented Mar 22, 2020 •

edited

Loading

typescript-bot commented Mar 22, 2020 •

edited

Loading

JasonHK Mar 22, 2020 •

edited

Loading