Change all apis on the Change, Data, all internal structures to be Symbols in order to not conflict with passed in data

[NullVoxPopuli]

Reproduction: https://runkit.com/nullvoxpopuli/reproduction-for-validated-changeset-122

for example,

import { Changeset as createChangeset } from 'validated-changeset';

let data = {
  trigger: { ...}
};

let changeset = createChangeset(data)

the data, trigger conflicts with: https://github.com/validated-changeset/validated-changeset/blob/ca0c7d249fc2244edaeb7aef25bdf06583628efa/src/index.ts#L144

I propose all internal APIS be assigned to private symbols instead of regular keys to not conflict with passed data:

example:

const TRIGGER = Symbol('__private__trigger__');

// ...
[TRIGGER](eventName: string, ...args: any[]): void {
  const notifier = notifierForEvent(this, eventName);

  if (notifier) {
    notifier.trigger(...args);
  }

[snewcomer]

This seems like a great idea! I'll get to working on this in the next few days!

[snewcomer]

Or of course, PRs are welcome!

[snewcomer]

@NullVoxPopuli A few thoughts.

1. How does one supplant and override the entire trigger method with their own logic? Is this one tradeoff we have to deal with? Perhaps a more applicable example is if somebody wanted to describe how they determine isInvalid and do not want to use our get isInvalid.
2. The real issue is we don't return the "getters/setters/methods" on the instance last. It happens somewhere before the end. I need to check the implications of pushing this as the last thing we do (and tests that may be impacted)

[NullVoxPopuli]

1. The symbols could be exported? So that way, if someone wants to subclass bufferedchangeset (ember), they could Import those symbols and define the appropriate overrides
2. Understandable, but I think the approach of falling back to internal stuff last would then mean that passed in data could break behavior (unless symbols are used for internal stuff)

[NullVoxPopuli]

Another possible approach is the change the api a bit, which would be breaking. The root of the issue right now is that changeset[property] proxies to the underlying data. If we forced using some other property and proxy that, problem solved. Similar to .get, except not using get. Like, changeset.current.property

[snewcomer]

Class based inheritance should get us all the functionality we need. Prioritizing the following is probably all users need.

1. Changes
2. Content (original data pased to e-c)
3. Changeset defined properties.

Can you help identify why the above prioritization might not work for a user? If we determine this is a bug (which I think it is - we prioritize Changeset over Content), then we can simply fix it.

We could use something like __methods = Symbol.for('ember-changeset-methods') and export that

export class EC {
  [__methods]: {
    toString
    trigger
    ...
  }
}

But my feeling is class based inheritance is sufficient in this case as long as we have the right prioritization order. Would love to hear your thoughts though!

123

[snewcomer]

Well giving #123 a go!

[snewcomer]

Alright I'm back to this issue 😆 . I completely borked the last PR b/c I thought my partner tests were passing. whispers they weren't.

I'll take a look at this during the week.