make modifier property non-enumerable about typebox

[sinclairzx81]

Hey, alright cool. Symbol as a key it is :) Will also rename to modifierSymbol as per suggestion and push to npm later this evening.

It's a nice update this one. Thanks again for the suggestion!

Cheers
S

[sinclairzx81]

Hey, that's a great idea :)

I have just pushed an update into the feature/symbol-modifier branch that takes the Symbol approach you suggested. You're right that using a Symbol omits the property from the resulting JSON.stringify which is certainly desirable (leaving a clean standard schema when serialised).

Ill leave this in the branch for you to have a quick review of the updates. If you're happy there (or have amendments you would like to see) happy to review those, otherwise, can publish these changes through later today.

Thanks for the suggestion !

[geekflyer]

Heyho, thanks for the quick response. I looked at your branch and I guess this could work but what I actually meant is not to use a symbol for the modifier value, but a symbol for the modifier property key.

So for example instead of:

  /** Modifies the inner type T into an optional T. */
  public static Optional<T extends TSchema | TUnion | TIntersect>(item: T): TOptional<T> {
    return { ...item, modifier: 'optional' }
  }

you could:

const ModifierSymbol = Symbol('TypeBoxModifier');

  /** Modifies the inner type T into an optional T. */
  public static Optional<T extends TSchema | TUnion | TIntersect>(item: T): TOptional<T> {
    return { ...item, [ModifierSymbol]: 'optional' }
  }

This would hide the property also from enumeration on the plain JavaScript object, not only from the serialized JSON.

If you run into problems with this approach (I vaguely remember TypeScript has in some edge-cases problems with typing symbol property names, but not quite sure if that's still the case), you could also just define non enumerable property, such as:

const ModifierKey = '__TypeBoxModifier';

  /** Modifies the inner type T into an optional T. */
  public static Optional<T extends TSchema | TUnion | TIntersect>(item: T): TOptional<T> {
    const optionalizedSchema = { ...item };
    Object.defineProperty(optionalizedSchema, '__TypeBoxModifier', {
      enumerable: false,
      value: 'optional'
    }
    return optionalizedSchema; 
  }

In both cases it's probably also a good idea to export the ModifierSymbol or ModifierKey so that users of typebox can access the modifier if they really want to (may be useful for some custom stuff etc.).

[sinclairzx81]

Hey, thanks for the follow up.

I checked in a an extra branch feature/symbol-modifier-key that uses the Symbol as a Key as per suggestion. I do kinda like that the modifier property remains enumerable in some capacity (I feel this is probably less surprising for the end user), but given the nature of the thing, I'm open to it being a non-enumerable key as well.

If you feel the modifier is best expressed as a symbol-key, can roll that branch through instead. As before, the implementation is there for quick review, can publish it through later tonight if all looks good.

Cheers
S

[geekflyer]

Ok nice. Checked out the symbol as key branch it looks good to me! Just had a minor comment on the symbol variable name.

Overall I kinda prefer the symbol as key pattern because that's what I think is relatively common - I have seen this particular pattern of using symbols in a couple of other libraries and some of the intros to symbols mention that as well.
The fact that keys which have a symbol value (instead of symbol key) get elided in JSON.stringify() was actually kinda surprising to me, so personally that's another reason I prefer the subjectively lesser "magic" of the symbol as key solution.

Thanks again for the quick response!
Cheers Chris

[sinclairzx81]

Updates published on @sinclair/[email protected] :)

Cheers !