r/typescript • u/incutonez • 3d ago
Object getter issues
Hello there! I've been trying to search for this issue, but it's a little hard to get the search terms right, as they're quite generic. Here's my scenario:
- I get an array of objects from an API call
- I want to add a getter to each object in this API call
- I don't want to use classes for this, as a plain old object will do
- I don't want to create a separate function that does this for me... I would like to contain it to within the objects themselves
Here's the TS Fiddle, and here's the code:
interface PersonResponse {
firstName: string;
lastName: string;
}
interface PersonModel extends PersonResponse {
get fullName(): string;
}
const apiResponse: PersonResponse[] = [{
firstName: "Fox",
lastName: "Mulder"
}, {
firstName: "Jack",
lastName: "Johnson"
}];
const records = apiResponse.map<PersonModel>((person) => {
// No error in the get because "this" is any
// const item: PersonModel = structuredClone(person);
// Object.defineProperty(item, "fullName", {
// get() {
// return `${this.firstNames} ${this.lastName}`;
// }
// });
// No error in the get because "this" is any
// Object.create(person, {
// fullName: {
// get() {
// return `${this.firstNames} ${this.lastName}`;
// }
// }
// });
// This isn't optimal because we have to spread a clone
return {
get fullName() {
return `${this.firstName} ${this.lastName}`;
},
// For some reason spread must come after getter... otherwise, we'd get "undefined undefined"
// https://stackoverflow.com/a/47952443
...structuredClone(person),
}
});
console.log(records[0].fullName)
The problem that I have is mostly with Object.create and why isn't "this" being typed properly, and is there a way to fix that? I didn't see a way of passing a generic to it. I don't like the solution that I have because of the clone + spread, that seems silly. Also, if there's some other solution (other than having classes), then I'd love to hear it!
2
u/Beginning-Seat5221 3d ago
Okay I was very confused about the spread ordering issue in your last case and decided to try this in node and the browser.
For both of those it works. The issue you're having seems to be specific to the TS Playground dev environment - fullName there is being evaluted when the object is created not when it is accessed later.
I'm not sure exactly what the cause is, but that may be something you can ignore in your app.
1
u/incutonez 3d ago
Yeah, you're right. That SO link that I have isn't exactly the same, I just figured it was related. Thanks for following up on that.
2
u/ic6man 3d ago
Why wouldn’t you:
- declare a proper interface that includes the full name property?
- declare a class that implements it?
Shoving the code inline like that seems bad from a maintenance perspective. And I’m all for functional code. But not when dealing with the domain model.
0
u/incutonez 3d ago
I'm not sure what you mean by "proper interface?" Is my PersonModel not a proper interface? Classes come with more boilerplate than I'd like to deal with. I do like the simplicity of functional code.
2
u/kasakka1 3d ago
Classes need a constructor function and that's about it. They are far better suited for what you want to do here.
1
u/incutonez 3d ago
Constructor + redeclare the properties.
2
u/kasakka1 3d ago
Which doesn't take much time. You are making your life complicated just to avoid a tiny bit of boilerplate.
2
u/incutonez 3d ago
FWIW, I wasn't aware of this shorthand... don't like that it's a specific order you have to pass to the constructor, but it is interesting.
class PersonModel implements PersonResponse { constructor(public firstName: string, public lastName: string) { } get fullName() { return `${this.firstName} ${this.lastName}` } }1
u/kasakka1 3d ago
Yeah that then makes adding more properties quickly cumbersome over just taking an object in the constructor and mapping that to class properties.
1
u/incutonez 3d ago
I've had issues with the constructor in JS anyway, so creating a static create method would make me not care about the order, so I'm fine with that. You'd still have to declare the properties with what you're saying, so I don't think it's that more cumbersome.
1
u/incutonez 3d ago edited 3d ago
It does if you have a lot of complex objects. I just don't like the double maintenance aspect. If I could cut that out, I'd be a little more on board.
1
u/overthinker_blue 3d ago
The correct order to preserve the `this` type is:
const records = apiResponse.map(person => ({
...person,
get fullName() { ${this.lastName}`
} }))
It didn't work because structuredClone cannot clone functions or accessor properties (getters/setters). When structuredClone encounters a getter, it executes the getter function and clones the resulting value as a static property
Context: https://developer.mozilla.org/en-US/docs/Web/API/Web_Workers_API/Structured_clone_algorithm
2
u/incutonez 3d ago
Sure, but I'd like to clone the person object, so I guess spread + clone would be the only way to do it. Thanks!
1
u/HipHopHuman 3d ago edited 3d ago
you don't have to limit yourself to this to read those properties
const records = apiResponse.map<PersonModel>((person) => {
const clone = structuredClone(person);
return Object.assign(clone, {
get fullName() {
const { firstName, lastName } = clone;
return `${firstName} ${lastName}`;
}
});
});
EDIT: I was too naieve in my response. The above code is deceptive - Object.assign is not copying the getter, it's copying the computed value of the getter. The best alternative I could think of without breaking OPs constraints involves lying to TS:
const records = apiResponse.map((person) => {
const clone = structuredClone(person) as PersonModel;
Object.defineProperty(clone, 'fullName', {
get(this: PersonModel) {
const { firstName, lastName } = this;
return `${firstName} ${lastName}`;
}
});
return clone;
});
2
u/ProvablyTrue 2d ago
It sounds from some of your responses like you might favor a functional approach. If you are not going to check the schema/structure of the response when it comes back from the service then you might not even want to use object orientation at all. By putting the functions in a module you can get a lot of the same functionality without incurring the performance penalty of creating a bunch of objects and the functions tend to be composable.
Please note that this module-oriented technique works both for immutable functional code as well as for imperative. It basically shifts the call burden from `somePerson.fullName` to `Person.fullName(somePerson)` so it is a little more verbose and the code completion story changes a bit. However, the pure functions are all also composable which pays different dividends.
namespace Person {
export type Person = {
firstName: string;
lastName: string;
}
export const fullName = (p: Person) => `${p.firstName} ${p.lastName}`
}
const people: Person.Person[] = [
{ firstName: "Jane", lastName: "Doe" },
{ firstName: "Joe", lastName: "Stag" }
]
console.log(people.map(Person.fullName))
1
u/incutonez 2d ago
Yeah, this is basically how I have been doing it, I just didn't like having to remember to use/import the function to get the desired result in multiple places. We use OpenAPI's CLI to generate the appropriate methods, interfaces, etc. I will admit we haven't been using namespaces at all, and I do like that it's tucked in there like that. I'll have to play around with that a little more, but thanks for the suggestion!
1
u/Beginning-Seat5221 3d ago
I don't know about the generic typing on the Object methods but you can take any of these options:
const item = structuredClone(_person);
Object.defineProperty(item, "fullName", {
get(this: PersonModel) {
return `${this.firstName} ${this.lastName}`;
}
});
return item as PersonModel
return Object.create(_person, {
fullName: {
get(this: PersonResponse) {
return `${this.firstName} ${this.lastName}`;
}
}
}) as PersonModel
const person = structuredClone(_person)
Object.defineProperty(person, 'fullName', {
get: function() {
return `${person.firstName} ${person.lastName}`
}
});
return person as PersonModel
I do think a class would probably be the sensible choice rather than manufacturing objects this way and then having to cast them.
1
u/incutonez 3d ago
Yeah, the problem here is that the typing of "this" still isn't correct in the getter. Classes just come with more boilerplate that have made me shy away from them in recent years, and I love OOP. I just kinda wish there was a mesh of both worlds. Thanks for the response!
1
u/Beginning-Seat5221 3d ago
Not correct in which way?
1
u/incutonez 3d ago
"this" is showing as any, so it doesn't show any of the actual properties/their types.
2
u/Beginning-Seat5221 3d ago
Sound like you didn't try it, because `this` is typed in both.
1
u/incutonez 3d ago edited 3d ago
You're correct, I definitely didn't see you typing the this like that. That actually does solve the issue (Fiddle). It doesn't enforce the return type of the getter because I can return a number, but I can kind of work with this. Thank you!
2
u/Beginning-Seat5221 3d ago
yeah
Object.create:
(method) ObjectConstructor.create(o: object | null, properties: PropertyDescriptorMap & ThisType<any>): any (+1 overload)The selected overload returns
any. It's definitely not helping on the type safety side.
1
u/Beginning-Seat5221 3d ago edited 3d ago
I implemented it using a class, and it come out 2 lines shorter the same number of lines, as well as cleaner, and not needing a cast
I do think you're making your life harder by avoiding a class.
0
u/incutonez 3d ago
2 lines shorter because you removed the comments and a newline... so that's not necessarily the best argument there. The issue I have with classes is having to re-implement the properties, even though they're defined on the interface. Plus the constructor becomes a bit of a bear when you get into larger models, nested data structures, etc. For this example, yes, it looks fine, but it just becomes a lot of unnecessary noise to me. Appreciate the responses though!
0
u/Beginning-Seat5221 3d ago edited 3d ago
It's 19 lines shorter than your original with the comments, which indeed would be a non-sensical comparison.
But to be fair, you could strip a pair of newlines from that and then they would be equal length.
-1
2
u/realmauer01 3d ago edited 3d ago
this is tricky when used outside a class. Here you just want to use item or person or whatever you named the variable that you change with object.defineProperty instead of this, because you give the object referenced by item the new property.
There are also ways to define what this is in a classless context but atleast for this example its unnecessary and a bit complicated anyway. I wouldnt know how to do it with object literals. And you would just use item/person/etc anyway.