Derived constructors should not be allowed to return primitives

[dhrubesh]

Q	A
Fixed Issues?	Fixes #13566
Patch: Bug Fix?
Major: Breaking Change?	No
Minor: New Feature?	👍
Tests Added + Pass?	👍
Documentation PR Link
Any Dependency Changes?
License	MIT

Derived constructor won't be allowed to return primitive values

[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 6f975ee:

Sandbox	Source
babel-repl-custom-plugin	Configuration
babel-plugin-multi-config	Configuration

[babel-bot]

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/47406/

[fedeci]

Looks fine to me.

[dhrubesh]

Hi @fedeci thanks for your comments, I've resolved them, please consider re-reviewing it. I'm not sure why my prettier on my local isn't running, I'll have to check that out.

[fedeci]

It is not running because those are strings and not "real lines of code". If I am not wrong we started moving the helpers to individual files so that eslint and prettier could work.

[existentialism]

Thoughts on matching v8's error?

Suggested change

	throw new TypeError("Invalid attempt to return primitive value");
	throw new TypeError("Derived constructors may only return object or undefined");

[nicolo-ribaudo]

This logic doesn't work if the constructor returns, for example, 0 or null (can you add a test for a falsy value?).

It should be something like

if (call && (typeof call === "object" || typeof call === "function")) {
  return call;
} else if (call !== void 0) {
  throw new TypeError("...");
}

[JLHwung]

Or

if (call !== void 0) {
  if (typeof call === "object" || typeof call === "function") {
    return call;
  } else {
    // throw errors
  }
}

[nicolo-ribaudo]

My version is 3 bytes shorter when using terser 😛

function f(o){if(o&&("object"==typeof o||"function"==typeof o))return o;if(void 0!==o)throw new TypeError("...")}
function g(o){if(void 0!==o){if("object"==typeof o||"function"==typeof o)return o;throw new TypeError("...")}}

Also, I think yours incorrectly allows returning null.

[KFlash]

but use of 'typeof' is a bad idea and kill perf.

[nicolo-ribaudo]

What do you suggest to use instead?

[KFlash]

Well. 'typeof obj === 'object';' is almost ~50% slower than other 'typeof'. And 'typeof number' return NaN even if it's not an number - in some cases.

There exist 2 popular utility libraries that has an optimized check for this.

Personally I would have used some kind of meta data and bitwise masks

[nicolo-ribaudo]

There exist 2 popular utility libraries that has an optimized check for this.

I only found is-object (which still uses typeof === "object") and lodash.isObject (which also uses typeof and checks if it's "object"). Why would typeof === "object" be slower than the other types?

Personally I would have used some kind of meta data and bitwise masks

o can be any possible JS value present in the JS code of our users, we cannot use bitwise mask

[KFlash]

I forgot the history behind this, but AngularJS had some good checks in their earlier versions. 1x of their library. I think they checked against the constructor. Also Google had a library a few years back - forgot the name - with fast checks.

[KFlash]

This 'else' is not needed. You already return in the line above.

[nicolo-ribaudo]

@dhrubesh Thanks! I merged this manually as 23d4f8c. I couldn't merge it using the GitHub UI because you made your changes on your fork's main branch rather than creating a new branch, so GitHub didn't let me update this PR to handle the review comments.