fetch icon indicating copy to clipboard operation
fetch copied to clipboard

`global` variable shadowed?

Open dosentmatter opened this issue 3 years ago • 2 comments

https://github.com/github/fetch/blob/d1d09fb8039b4b8c7f2f5d6c844ea72d8a3cefe6/fetch.js#L1-L5

I'm not sure if I'm reading this wrong, but it seems like the second global is shadowed by the var global declaration. After hoisting, the equivalent code is

var global; // default undefined
global =
  (typeof globalThis !== 'undefined' && globalThis) ||
  (typeof self !== 'undefined' && self) ||
  (typeof global !== 'undefined' && global) ||
  {}

Because of hoisting, the global gets evaluated as false in all environments:

(typeof global !== 'undefined' && global)
// becomes
(typeof undefined !== 'undefined' && global)
// becomes
('undefined' !== 'undefined' && undefined)
// becomes
(false &&  undefined)
// becomes
false

I think the intent was to access the global object, if it exists.

We might have to capture it to a variable, oldGlobal, first, but the hoisting of global prevents us from doing this easily. For example, the following wouldn't work because oldGlobal and global declarations are hoisted.

var oldGlobal = global; // === undefined
var global =
  (typeof globalThis !== 'undefined' && globalThis) ||
  (typeof self !== 'undefined' && self) ||
  (typeof oldGlobal !== 'undefined' && oldGlobal) ||
  {}

The only solution I can think of that preserves the variable name, global, is to wrap the code in an IIFE to prevent hoisting above oldGlobal. The typeof global is also moved up to oldGlobal to prevent errors on environments that don't global defined.

var oldGlobal = typeof global !== 'undefined' && global
(function () {
  var global =
    (typeof globalThis !== 'undefined' && globalThis) ||
    (typeof self !== 'undefined' && self) ||
    oldGlobal ||
    {}

  // rest of the code...
})()

An alternative solution is to rename global to some other name.

dosentmatter avatar Sep 10 '21 08:09 dosentmatter

Note that the behavior of var window or var global differ depending if you are in the global scope, module, or function scope.

Chrome console/snippet: This is in the global scope and window is already declared so var window does nothing.

var window = window;
window; // still the global window
(function () { var window; return window; })(); // undefined

Node.js repl: Also in the global scope.

var global = global;
global; // still the global object
(function () { var global; return global; })(); // undefined

Node.js file: Module scope so var global shadows the global-scoped global.

var global = global;
global; // undefined
(function () { var global; return global; })(); // undefined

Notice when the code gets wrapped in a function, window/global is consistently undefined. The fetch code does get wrapped in a function if we are using a module bundler like webpack.

dosentmatter avatar Sep 10 '21 09:09 dosentmatter

I'm wondering if this is causing error I have.

 Failed to execute 'fetch' on 'Window': Failed to read the 'signal' property from 'RequestInit': Failed to convert value to 'AbortSignal'.

i saw this other project doing an import : https://github.com/Azure/azure-sdk-for-js/pull/9880

oliverlj avatar May 03 '22 07:05 oliverlj