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

[TS] Basic support for functions in factories #695

Conversation

NLincoln
Copy link
Contributor

@NLincoln NLincoln commented Nov 2, 2020

There's a small limitation of the current types in that we can't use functions within factory definitions. This fixes it so that we can.

This should fix miragejs/examples#9 by making it so that we don't need to use the property-getter workaround

@samselikoff samselikoff merged commit ad007be into miragejs:master Nov 3, 2020
@samselikoff
Copy link
Contributor

Thank you!

@NLincoln NLincoln deleted the NLincoln-support-functions-in-factories-ts branch November 3, 2020 20:03
@timbakkum
Copy link

Hi, when will this be released? Latest version is from september.
We are migrating to TypeScript and use mirage factories for our unit tests as well, so we'd like to make them type-safe.

@samselikoff
Copy link
Contributor

@timbakkum Looks like master is breaking due to some failures with the TS type tests. If you have time and could look into those that would definitely help me get a release out faster!

@wagenet
Copy link
Contributor

wagenet commented Dec 2, 2021

I don't think this is correct. We have code like this:

export default Factory.extend({
  // This has to come before `id` so we have a value for `this.sha`
  sha(n: number): string {
    return `ABC${n}`;
  },

  id() {
    return `OirVnUtLCe_0:${this.sha}`;
  }
}

And previously, this.sha would report that it was a string. Now it claims to be string | ((n: number) => string) but when I add a debugger, it's still a string. It looks like the this is a special object that is created by iterating over each object in the hash passed to Factory.extend in order. This also means that the order of the properties matters as defining id will lead to the value of this.sha being undefined when id is called.

@wagenet
Copy link
Contributor

wagenet commented Dec 2, 2021

Ok, I see what's happening here now. I think this code is actually correct, but the this in the factory methods is not what is inferred.

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 this pull request may close these issues.

react-typescript example has the same data for each person
4 participants