Inherit Ent properties in pattern interface

[Swahvay]

Instead of doing

export interface ISomePattern {
  someField: string;
}

If you convert it to a type you can do

export type ISomePattern = Ent & {
  someField: string;
}

So that typing it as ISomePattern will still get you id and viewer. Plus it'll work in places that expect ents.

[Swahvay]

Ok, hear me out, I've thought about this some more and I have some ideas on how to make patterns even better.

So, instead of converting to type, leave it an interface and do interface ISomePattern extends Ent (I thought Ent was a class, not an interface). Now, convert this whole mixin to a "base" similar to how it's done with ents, so you would have SomePatternMixinBase. Then you can extend the base with an empty SomePatternMixin which will allow you to write custom methods on patterns.

[Swahvay]

I say leave ISomePattern as an interface, because if you're worried about backwards compatibility, you don't need to change the name to ISomePatternBase and then extend that in the other file. You can actually define it twice and TS will merge the two even if they don't reference each other. So:

// some_pattern_base.ts

// Don't export
interface ISomePattern extends Ent {
  someField: string;
}

// some_pattern.ts
export interface ISomePattern {
  myCustomFunc: () => void;
}

TS should just merge the two, so everywhere it will treat the interface as

export interface ISomePattern extends Ent {
  someField: string;
  myCustomFunc: () => void;
}

This works because we know the pattern file will always include the base file first. Kinda hacky, but it saves backwards compatibility and lets you customize pattern interfaces.

[lolopinto]

It’s mixins because IIRC, you cannot inherit from multiple base classes in typescript/javascript. Can you come up with examples of your proposal in a gist/playground so that it’s more concrete and less abstract?

…

On Fri, Feb 2, 2024 at 07:52 Stefan Parker ***@***.***> wrote: Ok, hear me out, I've thought about this some more and I have some ideas on how to make patterns even better. So, instead of converting to type, leave it an interface and do interface ISomePattern extends Ent (I thought Ent was a class, not an interface). Now, convert this whole mixin to a "base" similar to how it's done with ents, so you would have SomePatternMixinBase. Then you can extend the base with an empty SomePatternMixin which will allow you to write custom methods on patterns. — Reply to this email directly, view it on GitHub <#1739 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACS2VNYWVZQA2DTY7NUECXTYRUDSPAVCNFSM6AAAAABCHIFCY6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMRUGE2DSOBTHA> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[Swahvay]

It'll still be a mixin, but you'll be able to add custom fields/methods to the mixin.

https://gist.github.com/Swahvay/bc91257bdd89371e5b77bf12c7359705

[lolopinto]

https://github.com/lolopinto/ent/compare/fix-1739?expand=1

kinda works. can't reference base methods in other interface. thoughts?

[Swahvay]

Yeah, I realized there's no need to use the special overriding instance name hack here. You can simply name the original instance ISomePatternBase and have ISomePattern implements ISomePatternBase. That should allow you to reference base methods.

[Swahvay]

And it'll still be backwards compatible.

[Swahvay]

Also, the base interface should implement Ent too.

[lolopinto]

in practice, those set of changes don't work because FeedbackMixin doesn't have the methods IFeedbackBase requires. You should play with the entire chain in case i'm missing something

[lolopinto]

https://github.com/lolopinto/ent/compare/fix-1739?expand=1#diff-15737fb4db8aa691d60baeb60401b2860acd7ee9ad3a8cdae540cdc0158edc33R21

[Swahvay]

You are missing a couple things. For one, you need to change the Constructor type to be type Constructor<T extends Ent = Ent> = new (...args: any[]) => T;. Then you need to move the isPattern() out of the base and into the customizable one. But that's it I think. These two files work as expected:

import { Data, Ent, Viewer } from '@snowtop/ent';
import { Country, convertNullableCountry } from '../types';

type Constructor<T extends Ent = Ent> = new (...args: any[]) => T;

export interface ILocationBase extends Ent {
  address: string | null;
  address2: string | null;
  city: string | null;
  state: string | null;
  postalCode: string | null;
  country: Country | null;
}

function extractFromArgs<TViewer extends Viewer, TData extends Data>(
  args: any[],
): { viewer: TViewer; data: TData } {
  if (args.length !== 2) {
    throw new Error('args should be length 2');
  }
  return {
    viewer: args[0],
    data: args[1],
  };
}

export function LocationBaseMixin<T extends Constructor>(BaseClass: T) {
  return class LocationBaseMixin extends BaseClass implements ILocationBase {
    readonly address: string | null;
    readonly address2: string | null;
    readonly city: string | null;
    readonly state: string | null;
    readonly postalCode: string | null;
    readonly country: Country | null;
    constructor(...args: any[]) {
      super(...args);
      const { data } = extractFromArgs(args);
      this.address = data.address;
      this.address2 = data.address_2;
      this.city = data.city;
      this.state = data.state;
      this.postalCode = data.postal_code;
      this.country = convertNullableCountry(data.country);
    }
  };
}

import { Ent } from '@snowtop/ent';
import { ILocationBase, LocationBaseMixin } from './location_base';

type Constructor<T extends Ent = Ent> = new (...args: any[]) => T;

export function isLocation(ent: Ent): ent is ILocation {
  const o = ent as ILocation;
  return (o.isLocation && o.isLocation()) ?? false;
}

export interface ILocation extends ILocationBase {
  isLocation(): boolean;
}

export function LocationMixin<T extends Constructor>(BaseClass: T) {
  return class LocationMixin
    extends LocationBaseMixin(BaseClass)
    implements ILocation
  {
    isLocation() {
      return true;
    }
  };
}

[Swahvay]

I guess you also need to update implementations:

export class BarnInfoBase
  extends LocationMixin(class {} as (new (...args: any[]) => Ent))
  implements Ent<BarnlogViewer>, ILocation
{
 ...
}

(You could also put the Ent-fields inside the anonymous class, but I feel like that would be much harder to read and wouldn't really add anything)

[Swahvay]

I think your error was also stemming from this:

https://github.com/lolopinto/ent/compare/fix-1739?expand=1#diff-f1be87cfb1d9e732f22b07d00c5f336294cccf0f5f09f94866b742efc003f5d3R23

should be return class FeedbackBaseMixin extends BaseClass implements IFeedbackBase {

[lolopinto]

I think your error was also stemming from this:

https://github.com/lolopinto/ent/compare/fix-1739?expand=1#diff-f1be87cfb1d9e732f22b07d00c5f336294cccf0f5f09f94866b742efc003f5d3R23

should be return class FeedbackBaseMixin extends BaseClass implements IFeedbackBase {

i haven't had a chance to test the other things but it can't implement IFeedbackBase because it doesn't have implementations for everything else

[lolopinto]

I think your Location Pattern needs some edges so we can confirm that we're on the same page wrt behavior when those methods are missing from FeedbackMixin

[Swahvay]

You're right. You need to make one more subtle change. Each file needs to have its type Constructor definition point to the base interface.

type Constructor<T extends IFeedbackBase = IFeedbackBase> = new (...args: any[]) => T;

[lolopinto]

almost there

ent/examples/simple/src/ent/generated/mixins/feedback/actions/feedback_builder.ts

Lines 28 to 46 in 8f1f481

	// come back
	type Constructor<
	T extends IFeedback<ExampleViewerAlias> = IFeedback<ExampleViewerAlias>,
	> = new (
	...args: any[]
	) => T;
	interface BuilderConstructor<T extends IFeedback<ExampleViewerAlias>, C = {}> {
	orchestrator: Orchestrator<T, any, ExampleViewerAlias>;
	isBuilder<T extends Ent>(
	node: ID | T | Builder<T, any>,
	): node is Builder<T, any>;
	}
	export type FeedbackBuilderIsh<T extends IFeedback<ExampleViewerAlias>> =
	Constructor<
	// @ts-ignore TODO fix
	BuilderConstructor<T>
	>;

this needs to be fixed

[Swahvay]

I'm confused a little bit because I'm not getting any errors in my patterns with edges. What is the problem you're seeing?

[lolopinto]

would you consider this one fixed in v0.2.0?

[Swahvay]

I believe so. I've even started using it already. I think maybe a v0.3 feature might be to be able to add GraphQL fields to the pattern, but that would obviously require much more work and it obviously works the same by putting the fields on each ent.

[Swahvay]

It would also be nice if these patterned ents implemented interfaces in graphQL schemas, so that I could just do

... on MyPattern {
  someField
}

[Swahvay]

Closing this in favor of more specific lingering issues with patterns. Specifically #1832.