Mark class prototype as read-only

[wentout]

Q	A
Fixed Issues?	Fixes #2025
Patch: Bug Fix?	no
Major: Breaking Change?	not sure
Minor: New Feature?	yes
Tests Added + Pass?	test failed, WIP
Documentation PR Link
Any Dependency Changes?
License	MIT

This change is an attempt to follow the spec for .prototype property of class: necessity to follow writable: false.

The following repo describes that bug: https://github.com/denis-churbanov/babel-bug
There is also description of necessary fixes.
And here is Issue #2025 with exactly the same description.

And might be it worth looking to the following link: reactjs/rfcs#178
More detailed description here: facebook/react#4599 (comment)

But it is not about react team in general, it is much more related to Follow the Spec.
The spec purports the following code working:

const isClass = (fn) => {
	if (typeof fn !== 'function') {
		return false;
	}
	if (typeof fn.prototype !== 'object') {
		return false;
	}
	if (fn.prototype.constructor !== fn) {
		return false;
	}
	return Object.getOwnPropertyDescriptor(fn, 'prototype').writable === false;
};

And after transpilation everything becomes function, so...

And all we need to do to fix it is to add bit more code:

class WorkingClass {}
// addition right after class definition
Object.defineProperty(WorkingClass, 'prototype', {
    writable: false
});

So this change is about to bring some vision of solution.

[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 a054296:

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

[nicolo-ribaudo]

Since we already use an IIFE for classes, can we put the Object.defineProperty call inside of the iife?

[wentout]

Since we already use an IIFE for classes, can we put the Object.defineProperty call inside of the iife?

Yes, I'm working on it. Just not classes are not always wrapped with IIFE. Sometimes, when there is constructorOnly there is no IIFE. And also there are arrow functions, and they might return class. I'm trying to fix tests.

Current status it is Work in Progress, would it be better to convert it to draft or it is ok as it is?

[nicolo-ribaudo]

I have converted this to a draft, please mark it as ready when you need a review!

[nicolo-ribaudo]

What do you think about moving Object.defineProperty into the createClass helper, and always calling that helper even if there isn't any mehtod?

[wentout]

This PR is very outdated.

And, seems I don't know what to do. If we will fix this behavior, then seems something that relates on it may be broken.
I don't what and know how much is related, though.

Mostly I think of

Object.getOwnPropertyDescriptor(fn, 'prototype').writable === false;

[nicolo-ribaudo]

Mostly I think of

Object.getOwnPropertyDescriptor(fn, 'prototype').writable === false;

Isn't this exactly what this PR is trying to fix?
Btw, if you aren't interested in working on this anymore I can open another PR.

[wentout]

Mostly I think of

Object.getOwnPropertyDescriptor(fn, 'prototype').writable === false;

Isn't this exactly what this PR is trying to fix?
Btw, if you aren't interested in working on this anymore I can open another PR.

Sorry. I'm interested. Just was unable to find some time to finish.
If this is urgent now, so yes, it would be better with the other PR, otherwise please wait untill I figure out again, cause seems I will have more bright window soon. I think in a week or two.
:)

[nicolo-ribaudo]

Not urgent!

[justingrant]

What do you think about moving Object.defineProperty into the createClass helper, and always calling that helper even if there isn't any mehtod?

This seems like a good solution, IMHO. Seems easiest to implement and probably best for bundle size too.

This bug is a problem for anyone trying to polyfill ECMAScript built-in objects, because polyfills transpiled by Babel will have a writable prototype. Because the problem only shows up in transpilation, it's hidden from Test262 so is hard to discover for polyfill authors. We just bumped into this issue in the Temporal polyfill. tc39/proposal-temporal#1965 (comment)

Would be great to see a fix for this issue.

[babel-bot]

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

[JLHwung]

The writable defaults to false when a property is defined by Object.defineProperty. Since helper is shipped in user code. We can just call
Object.defineProperty(Constructor, "prototype", {}).

[ljharb]

Given that this function can be shimmed, and writability might not be supported (in es5-sham, it throws), this is a good change.

[nicolo-ribaudo]

It defaults to false only if the property does not exist yet:

function F() {}
Object.defineProperty(F, "prototype", {});
Object.getOwnPropertyDescriptor(F, "prototype").writable; // true