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.

Sign up for GitHub

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

Jump to bottom

doc: document globalThis TC39 stage 3 proposal #27399

Closed
wants to merge 3 commits into from
Closed

doc: document globalThis TC39 stage 3 proposal #27399

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9d042be
tools: fix node-core/required-modules eslint rule
bnoordhuis Apr 24, 2019
9bdd95f
doc: document globalThis TC39 stage 3 proposal
bnoordhuis Apr 24, 2019
cacb5f2
squash! s/REPLACEME/v11.0.0/
bnoordhuis Apr 25, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions doc/api/globals.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,18 @@ This variable may appear to be global but is not. See [`exports`].
## global
<!-- YAML
added: v0.1.27
deprecated: REPLACEME
-->

<!-- type=global -->

> Stability: 0 - Deprecated: Use [`globalThis)`][] instead.
Copy link
Member

Choose a reason for hiding this comment

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

Has the project decided to deprecate global now that globalThis is available?

I think there are still cases global is useful - in particular I've seen it used when probing for an environment that is Node.js and not "universal JavaScript".

Copy link
Member Author

Choose a reason for hiding this comment

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

I personally have never seen code that checks for the presence of global, it's always if (typeof process === 'object'), but doc-deprecating it won't break that.

The motivation for deprecation is that there should be One Way To Do It. globalThis seems strictly superior since it works in node and modern browsers.

Copy link
Member

@ljharb ljharb Apr 27, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Definitely don’t deprecate global; it’s so heavily used that it isn’t web compatible to name globalThis global :-)

In other words, there are many millions of examples in GitHub code search alone of code that does this.

Copy link
Member

@joyeecheung joyeecheung Apr 27, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

I agree it’s not possible to runtime deprecate global at this point but this document only doc-deprecate it? We may come back revisiting it or it could just stay doc deprecated forever but this sends a message to developers that they should use globalThis instead in new code.

Copy link
Member

Choose a reason for hiding this comment

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

I don’t think the message is needed. If they want to write universal code, they either can already rely on “every single bundler” handling global transparently for them, or they can use globalThis that works in both.

I just don’t think there’s any value or need to push people in a direction here.

Copy link
Contributor

Choose a reason for hiding this comment

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

This should also have a deprecation code filed in deprecations.md.


* {Object} The global namespace object.

## globalThis
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure we should document this as it's a JavaScript global.

Everything else in globals.md is a Node.js specific global that is not available in "plain" JavaScript. That is - nothing else here is defined in the ECMAScript spec (we don't list Array, String etc here for example).

Copy link
Member

Choose a reason for hiding this comment

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

well it's currently not in ecma262, it's still stage 3

Copy link
Member

Choose a reason for hiding this comment

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

I would suggest holding off for the time being

<!-- YAML
added: v11.0.0
-->

<!-- type=global -->
Expand Down
1 change: 0 additions & 1 deletion test/es-module/test-esm-basic-imports.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// Flags: --experimental-modules
/* eslint-disable node-core/required-modules */
import '../common/index.mjs';
import assert from 'assert';
import ok from '../fixtures/es-modules/test-esm-ok.mjs';
Expand Down
1 change: 0 additions & 1 deletion test/es-module/test-esm-cyclic-dynamic-import.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// Flags: --experimental-modules
/* eslint-disable node-core/required-modules */
import '../common/index.mjs';
import('./test-esm-cyclic-dynamic-import.mjs');
1 change: 0 additions & 1 deletion test/es-module/test-esm-double-encoding.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// Flags: --experimental-modules
/* eslint-disable node-core/required-modules */
import '../common/index.mjs';

// Assert we can import files with `%` in their pathname.
Expand Down
1 change: 0 additions & 1 deletion test/es-module/test-esm-encoded-path.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// Flags: --experimental-modules
/* eslint-disable node-core/required-modules */
import '../common/index.mjs';
import assert from 'assert';
// ./test-esm-ok.mjs
Expand Down
1 change: 0 additions & 1 deletion test/es-module/test-esm-forbidden-globals.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// Flags: --experimental-modules
/* eslint-disable node-core/required-modules */
import '../common/index.mjs';

// eslint-disable-next-line no-undef
Expand Down
2 changes: 0 additions & 2 deletions test/es-module/test-esm-import-meta.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
// Flags: --experimental-modules
/* eslint-disable node-core/required-modules */

import '../common/index.mjs';
import assert from 'assert';

Expand Down
1 change: 0 additions & 1 deletion test/es-module/test-esm-json-cache.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// Flags: --experimental-modules --experimental-json-modules
/* eslint-disable node-core/required-modules */
import '../common/index.mjs';

import { strictEqual, deepStrictEqual } from 'assert';
Expand Down
2 changes: 0 additions & 2 deletions test/es-module/test-esm-json.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
// Flags: --experimental-modules --experimental-json-modules
/* eslint-disable node-core/required-modules */

import '../common/index.mjs';
import { strictEqual } from 'assert';

Expand Down
2 changes: 0 additions & 2 deletions test/es-module/test-esm-live-binding.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
// Flags: --experimental-modules
/* eslint-disable node-core/required-modules */

import '../common/index.mjs';
import assert from 'assert';

Expand Down
1 change: 0 additions & 1 deletion test/es-module/test-esm-loader-invalid-format.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/loader-invalid-format.mjs
/* eslint-disable node-core/required-modules */
import { expectsError, mustCall } from '../common/index.mjs';
import assert from 'assert';

Expand Down
2 changes: 0 additions & 2 deletions test/es-module/test-esm-loader-invalid-url.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
// Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/loader-invalid-url.mjs
/* eslint-disable node-core/required-modules */

import { expectsError, mustCall } from '../common/index.mjs';
import assert from 'assert';

Expand Down
2 changes: 0 additions & 2 deletions test/es-module/test-esm-loader-missing-dynamic-instantiate-hook.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
// Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/missing-dynamic-instantiate-hook.mjs
/* eslint-disable node-core/required-modules */

import { expectsError } from '../common/index.mjs';

import('test').catch(expectsError({
Expand Down
1 change: 0 additions & 1 deletion test/es-module/test-esm-main-lookup.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// Flags: --experimental-modules
/* eslint-disable node-core/required-modules */
import '../common/index.mjs';
import assert from 'assert';

Expand Down
1 change: 0 additions & 1 deletion test/es-module/test-esm-named-exports.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/builtin-named-exports-loader.mjs
/* eslint-disable node-core/required-modules */
import '../common/index.mjs';
import { readFile } from 'fs';
import assert from 'assert';
Expand Down
2 changes: 0 additions & 2 deletions test/es-module/test-esm-namespace.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
// Flags: --experimental-modules
/* eslint-disable node-core/required-modules */

import '../common/index.mjs';
import * as fs from 'fs';
import assert from 'assert';
Expand Down
1 change: 0 additions & 1 deletion test/es-module/test-esm-process.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// Flags: --experimental-modules
/* eslint-disable node-core/required-modules */
import '../common/index.mjs';
import assert from 'assert';
import process from 'process';
Expand Down
1 change: 0 additions & 1 deletion test/es-module/test-esm-require-cache.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// Flags: --experimental-modules
/* eslint-disable node-core/required-modules */
import { createRequire } from '../common/index.mjs';
import assert from 'assert';
//
Expand Down
1 change: 0 additions & 1 deletion test/es-module/test-esm-shared-loader-dep.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/loader-shared-dep.mjs
/* eslint-disable node-core/required-modules */
import { createRequire } from '../common/index.mjs';

import assert from 'assert';
Expand Down
1 change: 0 additions & 1 deletion test/es-module/test-esm-shebang.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
#! }]) // isn't js
// Flags: --experimental-modules
/* eslint-disable node-core/required-modules */
import '../common/index.mjs';

const isJs = true;
Expand Down
1 change: 0 additions & 1 deletion test/es-module/test-esm-snapshot.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// Flags: --experimental-modules
/* eslint-disable node-core/required-modules */
import '../common/index.mjs';
import '../fixtures/es-modules/esm-snapshot-mutator.js';
import one from '../fixtures/es-modules/esm-snapshot.js';
Expand Down
2 changes: 0 additions & 2 deletions test/es-module/test-esm-throw-undefined.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
// Flags: --experimental-modules
/* eslint-disable node-core/required-modules */

import '../common/index.mjs';
import assert from 'assert';

Expand Down
1 change: 0 additions & 1 deletion test/es-module/test-esm-type-flag.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// Flags: --experimental-modules
/* eslint-disable node-core/required-modules */
import cjs from '../fixtures/baz.js';
import '../common/index.mjs';
import { message } from '../fixtures/es-modules/message.mjs';
Expand Down
2 changes: 1 addition & 1 deletion test/message/esm_display_syntax_error_import.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// Flags: --experimental-modules
/* eslint-disable no-unused-vars, node-core/required-modules */
/* eslint-disable no-unused-vars */
import '../common/index.mjs';
import {
foo,
Expand Down
1 change: 0 additions & 1 deletion test/message/esm_display_syntax_error_import_module.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// Flags: --experimental-modules
/* eslint-disable node-core/required-modules */
import '../common/index.mjs';
import '../fixtures/es-module-loaders/syntax-error-import.mjs';
1 change: 0 additions & 1 deletion test/message/esm_display_syntax_error_module.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// Flags: --experimental-modules
/* eslint-disable node-core/required-modules */
import '../common/index.mjs';
import '../fixtures/es-module-loaders/syntax-error.mjs';
11 changes: 11 additions & 0 deletions test/parallel/test-global-this.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
'use strict';

require('../common');
Copy link
Member

Choose a reason for hiding this comment

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

I am also not sure why we need a test for this as we don't have tests for other JavaScript features.

Copy link
Member Author

Choose a reason for hiding this comment

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

One reason is that it'll block this PR from accidentally getting back-ported to release branches that don't have globalThis. But really it's because I figured some extra sanity checks won't hurt.

const assert = require('assert');

assert.strictEqual(typeof globalThis, 'object');
assert.deepStrictEqual(
Object.getOwnPropertyDescriptor(globalThis, 'globalThis'),
{ configurable: true, enumerable: false, writable: true, value: globalThis });
assert.strictEqual(globalThis, global);
assert.strictEqual(String(globalThis), '[object global]');
10 changes: 10 additions & 0 deletions test/parallel/test-global-this.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// Flags: --experimental-modules
import '../common/index.mjs';
import assert from 'assert';

assert.strictEqual(typeof globalThis, 'object');
assert.deepStrictEqual(
Object.getOwnPropertyDescriptor(globalThis, 'globalThis'),
{ configurable: true, enumerable: false, writable: true, value: globalThis });
assert.strictEqual(globalThis, global);
assert.strictEqual(String(globalThis), '[object global]');
1 change: 0 additions & 1 deletion test/parallel/test-loaders-unknown-builtin-module.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/loader-unknown-builtin-module.mjs
/* eslint-disable node-core/required-modules */
import { expectsError, mustCall } from '../common/index.mjs';
import assert from 'assert';

Expand Down
4 changes: 4 additions & 0 deletions tools/eslint-rules/required-modules.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@ module.exports = function(context) {
* @returns {undefined|String} required module name or undefined
*/
function getRequiredModuleName(str) {
if (str === '../common/index.mjs') {
return 'common';
}

const value = path.basename(str);

// Check if value is in required modules array
Expand Down
1 change: 1 addition & 0 deletions tools/node_modules/eslint/node_modules/globals/globals.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.