Open Bug 1413167 Opened 7 years ago Updated 2 years ago

TreeView.js needs to stop modifying defaultProps

Categories

(DevTools :: Shared Components, enhancement, P3)

enhancement

Tracking

(Not tracked)

People

(Reporter: miker, Unassigned)

References

Details

As part of the migration to ES6 classes we declare defaultProps as a static getter e.g.

```
static get defaultProps() {
  object: null,
  renderRow: null,
  provider: ObjectProvider,
  expandedNodes: new Set(),
  expandableStrings: true,
  columns: []
}
```

This returns a new object each time defaultProps is accessed and ensures that we are not changing any props, which is bad practice.

In devtools/client/shared/components/tree/TreeView.js it turns out that we are somehow changing defaultProps.expandedNodes (it is easy to check by logging defaultProps within render).

As a temporary measure I have moved defaultProps outside of the getter so that the Component continues to work but when the issue is fixed we should move defaultProps back inside the component.

Normally I would suggest using Object.freeze() to diagnose this but it doesn't work on Set(). You can use this method to effectively freeze expandedNodes and find out where it is getting changed:

```
function freeze(obj, debug = false) {
  let name = obj.constructor.name;

  const debugThrow = msg => {
    if (debug) {
      // eslint-disable-next-line
      debugger;
    } else {
      try {
        throw new TypeError(msg);
      } catch (e) {
        let {message, stack} = e;

        stack = stack.split("\n")
                      .splice(2)
                      .join("\n");
        dump(`ERROR: ${message}\n`);
        dump(`STACK: ${stack}\n`);
        throw new Error(`See above for details`);
      }
    }
  };

  const getResult = (target, key) => {
    let result = Reflect.get(target, key);
    if (typeof result === "function") {
      result = result.bind(target);
    }
    return result;
  };

  switch (name) {
    case "Set":
      // Fall through.
    case "Map":
      return new Proxy(obj, {
        get(target, key) {
          if (key === "add" || key === "clear" || key === "delete" || key === "set") {
            debugThrow(`${name}.${key}() cannot be called because it is frozen`);
          }
          return getResult(target, key);
        }
      });
    case "Date":
      return new Proxy(obj, {
        get(target, key) {
          if (key.startsWith("set")) {
            debugThrow(`${name}.${key}() cannot be called because it is frozen`);
          }
          return getResult(target, key);
        }
      });
  }
}
```
This should be fixed in devtools/client/shared/components/tree/TreeView.js
Product: Firefox → DevTools
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.