use void 0 for undefined

[fsubal]

related: https://github.com/vercel/swr/pull/1150/files#diff-a9f5a8b144cf49d7b58fc30f932087e24c4c11d1e741d7d1c8be1a2f6d81fe71R2

src/utils/helper.ts claims that undefined could be replaced by something else so it has to be avoided.
It's correct if it runs in sloppy mode, but the current implementation ({})[0] can still be replaced to something else.
( only if Object.prototype[0] is mutated before importing swr, though )

({})[0] // undefined

Object.prototype[0] = 'something else'

({})[0] // "something else"

It seems ({})[0] is not the best approach to avoid it, this PR uses void 0 instead

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit a26ae43:

Sandbox	Source
SWR-Basic	Configuration
SWR-States	Configuration
SWR-Infinite	Configuration
SWR-SSR	Configuration

[shuding]

Thanks for noticing this and it's a very good point. We will address this after some minifier work!

Actually I'd propose an alternative way which has shorter code and it's safer.

[shuding]

Here's what I'm thinking about:

export const noop = () => {}

// The global value `undefined` can possibly be replaced by something else.
// This is a trick to get the safe `undefined` with shortest code.
export const UNDEFINED: undefined = (noop as any)[noop as any]

export const isUndefined = (v: any): v is undefined => v === UNDEFINED
export const isFunction = (v: any): v is Function => typeof v === 'function'
export const mergeObjects = (a: any, b: any) => Object.assign({}, a, b)

So the minified code will look like x[x], where x is an anonymous function.

The reason we are not using void 0 here is, the compiler currently inlines all UNDEFINED values to void 0 so the minified code will be ~100 bytes larger.

[fsubal]

The alternative way above would be shorter, but does not seem safer.

noop[noop] is still replaceable, since index accessing actually calls toString(), and all string key in objects are potentially pollutable if the key is not set.

const noop = () => {}

noop[noop] // undefined

Function.prototype['() => {}'] = 1

noop[noop] // 1

If you prioritize smaller bundle more than avoiding pollution, maybe this PR can be closed.

the compiler currently inlines all UNDEFINED values to void 0

I didn't come up with it. It makes sense.

---

[EDIT]

For taking both smaller bundle and preventing pollution, I came up with something like this.

This needs longer code to assign, but codes accessing this could be minified to x._, right ?

const UNDEFINED = Object.freeze({ _: void 0 })

UNDEFINED._ // undefined

UNDEFINED._ = 1
UNDEFINED._ // undefined

Object.prototype._ = 1
UNDEFINED._ // undefined

But I'm not sure it's technically "safe" and how acceptable the overhead would be ( help wanted ).

---

[EDIT 2]

Or, does this avoid inlining UNDEFINED to void 0 ?

const UNDEFINED = ({ 0: void 0 })[0]

[huozhi]

@fsubal you can use NaN[0] to replace with {}[0] since varaiables like NaN, Infinity can't be mutated or assigned with properties. And only give 1 byte increased to dist bundle.

[fsubal]

@huozhi No, NaN[0] can actually be overridden by mutating Number.prototype[0].

NaN[0] === undefined // true

Number.prototype[0] = 1

NaN[0] === 1 // true

[fsubal]

It seems that the way we can have is either of

• Getting more bundle size using void 0 ( this PR )
• Configuring bundler (if possible) to prevent inlining from UNDEFINED to void 0
• Accepting potential prototype pollution using something[0] ( the former state )
• Accepting runtime overhead to prevent global mutation using Object.freeze

[huozhi]

I feel we need to find a way to avoid void 0 to be inlined as undefined on minifier side. Then adopting this change will be safe.

[nstepien]

Is this really an issue in esm/strict mode?

[fsubal]

@nstepien The problem here is not that global undefined can be overridden, but using some key access of empty object as an alternative of undefined (for the sake of bundle size).

[nstepien]

@fsubal

for the sake of bundle size

I see, I wonder if any improvement in that aspect would already be covered by gzip/brotli compression. 🤔

[fsubal]

To summarize the bundle size problem ( from my understanding ), currently merging this PR would lead to something like this. @nstepien

When we have this source,

import { UNDEFINED } from 'src/utils/helpers'

// meaningless code but is useful for explaination
if (something() === UNDEFINED) {
  console.log(UNDEFINED, UNDEFINED, UNDEFINED, UNDEFINED)
}

We would get this,

// all undefined constants are inlined to void 0, which will make bundle size meaninglessly increase
if(a()===void 0){console.log(void 0,void 0,void 0,void 0)}

What we want instead is, for example,

// undefined is inlined only once, which could be acceptable
var x=void 0;if(a()===x){console.log(x,x,x,x)}

ref: #1372 (comment)

I think we have reached to the conclusion that we should change (or pursue) how to configure minifier of this project to accomplish the latter one, before merging this PR. #1372 (comment)

Once it's accomplished, we can take the smaller bundle (like now) and avoid potential prototype pollution at the same time.

[shuding]

Hi all! I opened #1533 which uses noop() as the undefined value.