ent icon indicating copy to clipboard operation
ent copied to clipboard

Inherit Ent properties in pattern interface

Open Swahvay opened this issue 1 year ago • 21 comments

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 avatar Jan 23 '24 16:01 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 avatar Feb 02 '24 15:02 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.

Swahvay avatar Feb 02 '24 15:02 Swahvay

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 https://github.com/lolopinto/ent/issues/1739#issuecomment-1924149838, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACS2VNYWVZQA2DTY7NUECXTYRUDSPAVCNFSM6AAAAABCHIFCY6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMRUGE2DSOBTHA . You are receiving this because you are subscribed to this thread.Message ID: @.***>

lolopinto avatar Feb 02 '24 15:02 lolopinto

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

Swahvay avatar Feb 02 '24 16:02 Swahvay

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

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

lolopinto avatar Feb 04 '24 05:02 lolopinto

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 avatar Feb 04 '24 13:02 Swahvay

And it'll still be backwards compatible.

Swahvay avatar Feb 04 '24 13:02 Swahvay

Also, the base interface should implement Ent too.

Swahvay avatar Feb 04 '24 13:02 Swahvay

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 avatar Feb 04 '24 16:02 lolopinto

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

lolopinto avatar Feb 04 '24 16:02 lolopinto

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 avatar Feb 04 '24 22:02 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 avatar Feb 04 '24 22:02 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 {

Swahvay avatar Feb 05 '24 01:02 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 {

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 avatar Feb 05 '24 02:02 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

lolopinto avatar Feb 05 '24 02:02 lolopinto

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;

Swahvay avatar Feb 05 '24 17:02 Swahvay

almost there

https://github.com/lolopinto/ent/blob/8f1f4814a68edbab3a215abd82e76af10c889108/examples/simple/src/ent/generated/mixins/feedback/actions/feedback_builder.ts#L28-L46 this needs to be fixed

lolopinto avatar Feb 06 '24 07:02 lolopinto

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?

Swahvay avatar Feb 06 '24 18:02 Swahvay

would you consider this one fixed in v0.2.0?

lolopinto avatar Mar 02 '24 06:03 lolopinto

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 avatar Mar 02 '24 21:03 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 avatar Mar 02 '24 21:03 Swahvay

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

Swahvay avatar Sep 06 '24 02:09 Swahvay