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

Convert isPattern functions to TS user-defined type guards #1732

Closed
Swahvay opened this issue Jan 18, 2024 · 5 comments · Fixed by #1735
Closed

Convert isPattern functions to TS user-defined type guards #1732

Swahvay opened this issue Jan 18, 2024 · 5 comments · Fixed by #1735

Comments

@Swahvay
Copy link
Contributor

Swahvay commented Jan 18, 2024

Instead of simply doing

export interface ISomePattern {
  someID: ID;
}

export function SomePatternMixin<T extends Constructor>(
  BaseClass: T,
) {
  return class SomePatternMixin extends BaseClass {
    readonly someID: ID;
    constructor(...args: any[]) {
      super(...args);
      const { data } = extractFromArgs(args);
      this.someID = data.some_id;
    }

    isSomePattern() {
      return true;
    }
  };
}

The isSomePattern can type the variable when it is called by changing the call signature.

isSomePattern(): this is ISomePattern {
  return true;
}

This would then allow you to do someEnt?.isSomePattern() && someEnt.someID without type errors.

@Swahvay
Copy link
Contributor Author

Swahvay commented Jan 19, 2024

Actually, I guess for that to work, you'd also have to default every ent to include isSomePattern to return false. Is there a good way then to get if an ent is an instance of a pattern?

@lolopinto
Copy link
Owner

I think https://github.com/lolopinto/ent/compare/fix-1732?expand=1 is the best I can do (expand the PR to see manual changes)

and then you'll have to call isSomePattern(ent) and trust that if it meets the criteria, that's good enough

if you somehow have a different pattern or an ent which has that same shape, it'll pass and then fail in a different way...

we can cheat and add a new field that's only unique to objects which have that pattern name which should heavily increase the odds of it returning the right thing

@Swahvay
Copy link
Contributor Author

Swahvay commented Jan 21, 2024

What if, instead of

const o = (obj as unknown as IWithAddress);
return 'addressId' in o && typeof o.addressId === 'string' || typeof o.addressId === 'number' || o.addressId === null;

You kept the isWithAddress function on the pattern/ent, then you could do

const o = (obj as unknown as IWithAddress);
return o.isWithAddress?.();

@lolopinto
Copy link
Owner

we can do that but it doesn't guarantee anything

would still have this same issue

and then you'll have to call isSomePattern(ent) and trust that if it meets the criteria, that's good enough

happy to make that change tho cos it's easier :)

@Swahvay
Copy link
Contributor Author

Swahvay commented Jan 22, 2024

Yeah, I get that it's never possible to do this with 100% safety. Figured this was at least a little more direct about it though.

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

Successfully merging a pull request may close this issue.

2 participants