Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid using Symbol.for('ok-computer.object-root') #28

Open
richardscarrott opened this issue Sep 23, 2022 · 0 comments
Open

Avoid using Symbol.for('ok-computer.object-root') #28

richardscarrott opened this issue Sep 23, 2022 · 0 comments

Comments

@richardscarrott
Copy link
Owner

In many cases, you want to be able to serialize errors (usually w/ JSON.stringify), however currently the object root property is lost as it's a symbol.

  const user = object({ name: string });
  const error = user({ name: 123, foo: 'not allowed' });

  console.log(error);
  // {
  //  name: 'Expected typeof string',
  //  [Symbol(ok-computer.object-root)]: 'Unknown properties "foo"'
  // }

  console.log(JSON.stringify(error));
  // { "name": "Expected typeof string" }

It was originally a string (__root) but was changed to a symbol to avoid clashes with user properties. In hindsight I think it's worth going back to a string property and a) giving it a better namespace and b) throwing if a validator is passed in with a conflicting name.

e.g.

const user = object({ name: string });
const error = user({ name: 123, foo: 'not allowed' });

 console.log(error);
 // {
 //  name: 'Expected typeof string',
 //  'ok-computer.error': 'Unknown properties "foo"'
 // }

console.log(JSON.stringify(error));
// { "name": "Expected typeof string" }
  
object({ 'ok-computer.error': string }); // throw

NOTE: As a reminder, we return a "root" error as a property like this (rather than { name?: string } | string), so the returned interface is always an object making it easier to consume).

Implementation details

  1. This change may mean listErrors no longer needs to use ownEnumerableEntries https://github.com/richardscarrott/ok-computer/blob/master/src/ok-computer.ts#L76
  2. May want to avoid calling it 'root' now it's used for both "Expected object" and "Unknown properties foo"
  3. The same needs to be done for array; although arrays obviously don't non-index properties...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant