Avoid adding props to global function prototype. Doing so breaks the react-three-fiber reconciler
Closes #9206
Change List
- create-props.ts Changed
createPropsPrototypeAndTypesto bail out if the parent class of the passed component class is the global function prototype so that it doesn't get polluted.
There may well be a better and cleaner way to do this, but this fixes the bug I'm experiencing.
Note that I was not able to get tests to run locally or on my fork via Github Actions, but the errors appear unrelated to my changes and seem to be occurring on some workflow runs on the main repo.
P.S. deck.gl is an absolutely amazing piece of software!
There do seem to be some relevant failing tests, does yarn test browser not work for you locally?
yarn test browser does indeed work locally - thanks. I can see there's only one failing test if I roll back my change vs 25 failing tests with my change. I'll look further into them to see if I can understand the issue.
The failing tests I saw on the CI build related to missing image files. If I run yarn test locally, I get
file:///redacted/deck.gl/examples/layer-browser/src/data-samples.js:6
import allPoints from '../data/sf.bike.parking.json' assert { type: 'json' };
^^^^^^
SyntaxError: Unexpected identifier 'assert'
at compileSourceTextModule (node:internal/modules/esm/utils:337:16)
at ModuleLoader.moduleStrategy (node:internal/modules/esm/translators:166:18)
at callTranslator (node:internal/modules/esm/loader:437:14)
at ModuleLoader.moduleProvider (node:internal/modules/esm/loader:443:30)
Node.js v22.9.0
error Command failed with exit code 1.
Playing with this a bit further, It's starting to look like deck.gl requires attaching props to the global function prototype in order for the layer tests to pass - something that is incompatible with the way the react-three-fiber reconciler works. I need to spend more time familiarising myself with the deck.gl code and what role props play.
@marsy-invests Can you pinpoint in the debugger when the function prototype is polluted? Perhaps add a debugger statement to your if statement and see where it breaks and follow the stack trace?
@ibgreen if I change getPropsPrototype (see https://github.com/visgl/deck.gl/blob/master/modules/core/src/lifecycle/create-props.ts#L82) to throw an error when about to pollute the function prototype as follows:
function getPropsPrototype(componentClass, extensions) {
// A string that uniquely identifies the extensions involved
let cacheKey = MergedDefaultPropsCacheKey;
if (extensions) {
for (const extension of extensions) {
const ExtensionClass = extension.constructor;
if (ExtensionClass) {
cacheKey += `:${ExtensionClass.extensionName || ExtensionClass.name}`;
}
}
}
const defaultProps = getOwnProperty(componentClass, cacheKey);
if (!defaultProps) {
if (componentClass === Function.prototype) {
throw Error("About to pollute function prototype", componentClass)
}
return (componentClass[cacheKey] = createPropsPrototypeAndTypes(componentClass, extensions || []));
}
return defaultProps;
}
I get the following stack trace
ErrorBoundary.tsx:29 Uncaught error: Error: About to pollute function prototype
at getPropsPrototype (chunk-WVBUUDCH.js?v=94ea3cd1:34413:13)
at createPropsPrototypeAndTypes (chunk-WVBUUDCH.js?v=94ea3cd1:34425:30)
at getPropsPrototype (chunk-WVBUUDCH.js?v=94ea3cd1:34415:39)
at createPropsPrototypeAndTypes (chunk-WVBUUDCH.js?v=94ea3cd1:34425:30)
at getPropsPrototype (chunk-WVBUUDCH.js?v=94ea3cd1:34415:39)
at createPropsPrototypeAndTypes (chunk-WVBUUDCH.js?v=94ea3cd1:34425:30)
at getPropsPrototype (chunk-WVBUUDCH.js?v=94ea3cd1:34415:39)
at createProps (chunk-WVBUUDCH.js?v=94ea3cd1:34385:26)
at new Component (chunk-WVBUUDCH.js?v=94ea3cd1:34543:18)
at new Layer (chunk-WVBUUDCH.js?v=94ea3cd1:34918:5) {componentStack: '\n at Image (http://localhost:5173/src/ui/le…(http://localhost:5173/src/ErrorBoundary.tsx:6:1)'}
And if I add console.log("Calling getPropsPrototype for", componentClass) to the start of getPropsPrototype, here's the logs that are created as it walks the prototype chain as the layer gets created.
Calling getPropsPrototype for class extends CompositeLayer {
static {
this.layerName = "GeoJsonLayer";
}
static {
this.defaultProps = defaultProps15;
}
initializeState() {
this.state = {
layerProps: {},
…
Calling getPropsPrototype for class extends Layer {
static {
this.layerName = "CompositeLayer";
}
/** `true` if this layer renders other layers */
get isComposite() {
return true;
}
/** Returns true if all async res…
Calling getPropsPrototype for class extends Component {
constructor() {
super(...arguments);
this.internalState = null;
this.lifecycle = LIFECYCLE.NO_STATE;
this.parent = null;
}
static {
this.defaultProps = d…
Calling getPropsPrototype for class {
static {
this.componentName = "Component";
}
static {
this.defaultProps = {};
}
constructor(...propObjects) {
this.props = createProps(this, propObjects);
this.id = this.p…
Calling getPropsPrototype for ƒ () { [native code] }
chunk-NUM45GRF.js?v=586f8a51:16638 Uncaught Error: About to pollute function prototype
at getPropsPrototype (chunk-SL5OX4HQ.js?v=586f8a51:34414:13)
at createPropsPrototypeAndTypes (chunk-SL5OX4HQ.js?v=586f8a51:34426:30)
at getPropsPrototype (chunk-SL5OX4HQ.js?v=586f8a51:34416:39)
at createPropsPrototypeAndTypes (chunk-SL5OX4HQ.js?v=586f8a51:34426:30)
at getPropsPrototype (chunk-SL5OX4HQ.js?v=586f8a51:34416:39)
at createPropsPrototypeAndTypes (chunk-SL5OX4HQ.js?v=586f8a51:34426:30)
at getPropsPrototype (chunk-SL5OX4HQ.js?v=586f8a51:34416:39)
at createPropsPrototypeAndTypes (chunk-SL5OX4HQ.js?v=586f8a51:34426:30)
at getPropsPrototype (chunk-SL5OX4HQ.js?v=586f8a51:34416:39)
at createProps (chunk-SL5OX4HQ.js?v=586f8a51:34385:26)
Revised the fix. It now passes the test suite. getPropsPrototype now just returns an empty object if the componentClass passed is not actually a Component.