ember-changeset icon indicating copy to clipboard operation
ember-changeset copied to clipboard

ember-inspector silently pollutes `changes`

Open betocantu93 opened this issue 3 years ago • 6 comments

Changes are silently set to the changeset when ember-inspector is used (?) tracked it down to this line

https://github.com/emberjs/ember-inspector/blob/15f49e5ab04e738ec540550001ad8a1b1537eb70/ember_debug/object-inspector.js#L536

it triggers the proxy set and so changeset.changes is polluted.

In particular these properties:

_oldWillDestroy willDestroy

Maybe it would be great to have an inverse of changesetKeys, a denylist, which the default could be these props

betocantu93 avatar Apr 17 '21 04:04 betocantu93

A denyKeys is a good idea! Perhaps we add that with the default of those two keys. (I've looked at this problem in the past and haven't come up with a reasonable solution)

snewcomer avatar Apr 17 '21 16:04 snewcomer

Great catch @betocantu93

I was going crazy to find the reason. 👍

sinankeskin avatar Apr 24 '21 13:04 sinankeskin

I got around by using my own default changeset, and extend from it if I need further types

//changesets/default
import { EmberChangeset } from 'ember-changeset';

const DENY_LIST = '_denyListKeys';
const AFTER_CHANGE = 'afterChange';
export class DefaultChangeset extends EmberChangeset {
  [DENY_LIST] = ['_oldWillDestroy', 'willDestroy', '_super'];

  _setProperty({ key, value, oldValue }) {
    if (this[DENY_LIST].includes(key)) return;
    super._setProperty({ key, value, oldValue });
    //Adds an event for afterChange to hook as observer.
    this.trigger(AFTER_CHANGE, key)
  }
}

//helpers/changeset
import { helper } from '@ember/component/helper';
import { changeset as wrappedChangesetHelper } from 'ember-changeset/helpers/changeset';
import { DefaultChangeset } from '../changesets/default';

export function changeset(positional, options = {}) {
  let opts = {
    changeset: DefaultChangeset,
    ...options,
  };

  return wrappedChangesetHelper(positional, opts);
}

export default helper(changeset);

Probably worth a PR, because curretnly we only have an allowList through changesetKeys which means you need to pass in all keys to avoid this bug, so I went for the denylist approach. Probably changesetKeys could be also be renamed to allowKeys.

betocantu93 avatar Apr 24 '21 14:04 betocantu93

FWIW this is the same issue as https://github.com/poteto/ember-changeset/issues/485

patrickberkeley avatar May 19 '21 04:05 patrickberkeley

I am having this same issue, grateful if the denylist approach could be considered.

dreamsbond avatar Aug 18 '21 12:08 dreamsbond

deny list seems like a great idea if someone has the time to contribute!

snewcomer avatar Aug 18 '21 12:08 snewcomer