chore: Align isObject/isRecord usage #1132
Conversation
| } | ||
| } | ||
| }; | ||
| "intent" in source && typeof source.intent === "string" && |
There was a problem hiding this comment.
nit: isn't "..." in source redundant here, since isObject(source) already makes sure this is an object and typeof source.<..> would be "undefined" if the key doesn't exist?
There was a problem hiding this comment.
That's what I thought -- the Record type (where Record<number, unknown> could be an array) allows this but object types do not (and had to add it in a few places) -- maybe there's a better way to do this? but we do sometimes explicitly want an object-like ({}) things and not an array or a record
There was a problem hiding this comment.
We'd get a Property 'intent' does not exist on type 'object' otherwise
There was a problem hiding this comment.
hmm, works for me:
the main problem is that for x = null it'll throw. it's really weird, because it even works for x being a string or a number. but I think you'd be fine if isObject() is true.
(also, doesn't matter for this PR, this got me into TIL mode)
There was a problem hiding this comment.
We'd get a
Property 'intent' does not exist on type 'object'otherwise
This is a typescript check, and we check that x is not null (isObject(..)) and TS null is not object (unlike JS) -- usually we want those checks ("property does not exist"), though using isRecord previously allowed any key (TIL as well)
isObject(andisRecord) -- sometimes as "non-array" objects, sometimes including arrays, and the differences betweenobjectandRecordusage.@commontools/utils(originally tracking down a bug for something pulling in too many modules, but was misleading). Deno requires a root export, so a compile time (no exports) or runtime error fires if doing so to avoid inadvertantly pulling in all utilsSummary by mrge
Standardized all isObject and isRecord checks across the codebase by providing consistent utility functions in utils/types, and removed duplicate or inconsistent implementations.