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

feat: Add `globalThis` #40096

Merged
merged 1 commit into from Nov 5, 2019

Conversation

@ExE-Boss
Copy link
Contributor

ExE-Boss commented Nov 2, 2019

The globalThis polyfill implements a cross‑platform compatible implementation of the ES2020 globalThis property.


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).

If adding a new definition:

  • The package does not already provide its own types, or cannot have its .d.ts files generated via --declaration
  • If this is for an NPM package, match the name. If not, do not conflict with the name of an NPM package.
  • Create it with dts-gen --dt, not by basing it on an existing project.
  • Represents shape of module/library correctly
  • tslint.json should be present and it shouldn't have any additional or disabling of rules. Just content as { "extends": "dtslint/dt.json" }. If for reason the some 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.
  • tsconfig.json should have noImplicitAny, noImplicitThis, strictNullChecks, and strictFunctionTypes set to true.

review?(@ljharb)

// Project: https://github.com/ljharb/System.global#readme
// Definitions by: ExE Boss <https://github.com/ExE-Boss>
// Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped
// TypeScript Version: 3.4

This comment has been minimized.

Copy link
@ExE-Boss

ExE-Boss Nov 2, 2019

Author Contributor

Support for globalThis was added in TypeScript 3.4.

This comment has been minimized.

Copy link
@ljharb

ljharb Nov 2, 2019

Contributor

it’s the same type as windows or global`; were these types not in earlier versions?

This comment has been minimized.

Copy link
@ExE-Boss

ExE-Boss Nov 2, 2019

Author Contributor

There is declare var window, but that requires lib: ["dom"], and declare var global, which requires @types/node, but only the former one has, and this only exists in TypeScript 3.4+:

declare var window: Window & typeof globalThis;
declare var self: Window & typeof globalThis;

Node’s global only has:

declare var global: NodeJS.Global;

Also, even if you do:

var something;

Even though it will be available as window.something during runtime, without globalThis, the compiler wouldn’t know that.


Also, in lib.dom.d.ts, Window functions have to duplicated in the global scope, so that you can do:

addEventListener("foo", e => {
	// Process the `foo` event
})

Same with Node globals.

@typescript-bot

This comment has been minimized.

Copy link
Contributor

typescript-bot commented Nov 2, 2019

@ExE-Boss 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

This comment has been minimized.

Copy link
Contributor

typescript-bot commented Nov 2, 2019

👋 Hi there! I’ve run some quick measurements against master and your PR. These metrics should help the humans reviewing this PR gauge whether it might negatively affect compile times or editor responsiveness for users who install these typings.

Let’s review the numbers, shall we?

These typings are for a package that doesn’t yet exist on master, so I don’t have anything to compare against yet! In the future, I’ll be able to compare PRs to globalthis with its source on master.

Comparison details 📊
Batch compilation
Type count 1267
Assignability cache size 91
Language service measurements
Samples taken 8
Identifiers in tests 8
getCompletionsAtPosition
    Mean duration (ms) 79.3
    Mean CV 36.0%
    Worst duration (ms) 103.4
    Worst identifier shim
getQuickInfoAtPosition
    Mean duration (ms) 61.9
    Mean CV 22.2%
    Worst duration (ms) 65.8
    Worst identifier getGlobal
System information
Node version v10.16.3
CPU count 2
CPU speed 2.294 GHz
CPU model Intel(R) Xeon(R) CPU E5-2673 v4 @ 2.30GHz
CPU Architecture x64
Memory 6.8 GiB
Platform linux
Release 4.15.0-1059-azure
@ljharb
ljharb approved these changes Nov 2, 2019
Copy link
Contributor

ljharb left a comment

seems like there should be a better thing than typeof globalThis for older TS versions; perhaps typeof (function () { return this; }())?

@ExE-Boss

This comment has been minimized.

Copy link
Contributor Author

ExE-Boss commented Nov 2, 2019

@ljharb That won't work for several reasons:

  1. .d.ts files can’t have function bodies, since they just define the type signatures.
  2. The resulting type of (function () { return this; })(); is any.
@typescript-bot

This comment has been minimized.

Copy link
Contributor

typescript-bot commented Nov 5, 2019

We've gotten sign-off from a reviewer 👏. A maintainer will soon review this PR and merge it if there are no issues. If it shouldn't be merged yet, please leave a comment saying so and we'll wait. Thank you for contributing to DefinitelyTyped!

@sandersn sandersn merged commit 0410320 into DefinitelyTyped:master Nov 5, 2019
3 checks passed
3 checks passed
DefinitelyTyped.BenchmarkPR Build #22450 succeeded
Details
DefinitelyTyped.DefinitelyTyped Build #20191102.28 succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Pull Request Status Board automation moved this from Review to Done Nov 5, 2019
@ExE-Boss ExE-Boss deleted the EB-Forks:types/globalthis branch Nov 5, 2019
@typescript-bot

This comment has been minimized.

Copy link
Contributor

typescript-bot commented Nov 5, 2019

I just published @types/globalthis@1.0.0 to npm.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.