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.

Sign up for GitHub

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

Jump to bottom

feat(clone): enhance type and logic. #206

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
+127 −44
Open

feat(clone): enhance type and logic. #206

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2865f6b
feat(clone): enhance type and logic.
samchon Jul 15, 2024
8dd0bc2
Update src/object/clone.ts
samchon Jul 17, 2024
7131028
Enhance objet type cloning
samchon Jul 17, 2024
f28db42
Change `Equal` type of `clone()` function.
samchon Jul 17, 2024
e277e2d
For legacy NodeJS
samchon Jul 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 39 additions & 17 deletions src/object/clone.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,52 +1,52 @@
import { describe, expect, it } from "vitest";
import { clone } from "./clone";
import { describe, expect, it } from 'vitest';
import { clone, Shallowed } from './clone';

describe("clone", () => {
it("should return primitive values as is", () => {
const symbol = Symbol("symbol");
describe('clone', () => {
it('should return primitive values as is', () => {
const symbol = Symbol('symbol');

expect(clone(42)).toBe(42);
expect(clone("es-toolkit")).toBe("es-toolkit");
expect(clone('es-toolkit')).toBe('es-toolkit');
expect(clone(symbol)).toBe(symbol);
expect(clone(true)).toBe(true);
expect(clone(null)).toBe(null);
expect(clone(undefined)).toBe(undefined);
});

it("should clone arrays", () => {
it('should clone arrays', () => {
const arr = [1, 2, 3];
const clonedArr = clone(arr);

expect(clonedArr).toEqual(arr);
expect(clonedArr).not.toBe(arr);
});

it("should clone objects", () => {
const obj = { a: 1, b: "es-toolkit", c: [1, 2, 3] };
it('should clone objects', () => {
const obj = { a: 1, b: 'es-toolkit', c: [1, 2, 3] };
const clonedObj = clone(obj);

expect(clonedObj).toEqual(obj);
expect(clonedObj).not.toBe(obj);
});

it("should clone dates", () => {
it('should clone dates', () => {
const date = new Date();
const clonedDate = clone(date);

expect(clonedDate).toEqual(date);
expect(clonedDate).not.toBe(date);
});

it("should clone regular expressions", () => {
it('should clone regular expressions', () => {
const regex = /abc/g;
const clonedRegex = clone(regex);

expect(clonedRegex).toEqual(regex);
expect(clonedRegex).not.toBe(regex);
});

it("should shallow clone nested objects", () => {
const nestedObj = { a: [1, 2, 3], b: { c: "es-toolkit" }, d: new Date() };
it('should shallow clone nested objects', () => {
const nestedObj = { a: [1, 2, 3], b: { c: 'es-toolkit' }, d: new Date() };
const clonedNestedObj = clone(nestedObj);

expect(clonedNestedObj).toEqual(nestedObj);
Expand All @@ -55,26 +55,48 @@ describe("clone", () => {
expect(clonedNestedObj.a[2]).toEqual(nestedObj.a[2]);
});

it("should return functions as is", () => {
it('should return functions as is', () => {
const func = () => {};
const clonedFunc = clone(func);

expect(clonedFunc).toBe(func);
});

it("should clone sets", () => {
it('should clone sets', () => {
const set = new Set([1, 2, 3]);
const clonedSet = clone(set);

expect(clonedSet).toEqual(set);
expect(clonedSet).not.toBe(set);
});

it("should clone maps", () => {
const map = new Map([[1, "a"], [2, "b"], [3, "c"]]);
it('should clone maps', () => {
const map = new Map([
[1, 'a'],
[2, 'b'],
[3, 'c'],
]);
const clonedMap = clone(map);

expect(clonedMap).toEqual(map);
expect(clonedMap).not.toBe(map);
});

// check whether two types are equal only in the compiler level
let x: Shallowed<SomeClass> = null!;
let y: SomeInterface = null!;
x = y;
y = x;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this has to be tested with some other type testing library.

See vitest's docs for details.

});

declare class SomeClass {
public id: string;
public name: string;
public age: number;
public getName(): string;
}
interface SomeInterface {
id: string;
name: string;
age: number;
}
79 changes: 67 additions & 12 deletions src/object/clone.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,48 +26,103 @@
* console.log(clonedObj); // { a: 1, b: 'es-toolkit', c: [1, 2, 3] }
* console.log(clonedObj === obj); // false
*/
export function clone<T>(obj: T): T {
export function clone<T>(obj: T): Shallowed<T> {
samchon marked this conversation as resolved.
Show resolved Hide resolved
if (isPrimitive(obj)) {
return obj;
return obj as Shallowed<T>;
}

if (Array.isArray(obj)) {
return obj.slice() as T;
return obj.slice() as Shallowed<T>;
}

if (obj instanceof Date) {
return new Date(obj.getTime()) as T;
return new Date(obj.getTime()) as Shallowed<T>;
}

if (obj instanceof RegExp) {
return new RegExp(obj.source, obj.flags) as T;
return new RegExp(obj.source, obj.flags) as Shallowed<T>;
}

if (obj instanceof Map) {
const result = new Map();
for (const [key, value] of obj) {
result.set(key, value);
}
return result as T;
return result as Shallowed<T>;
}

if (obj instanceof Set) {
const result = new Set();
for (const value of obj) {
result.add(value);
}
return result as T;
return result as Shallowed<T>;
}

if (typeof obj === "object") {
return Object.assign({}, obj) as T;
// BINARY DATA
if (obj instanceof Uint8Array) return new Uint8Array(obj) as Shallowed<T>;
if (obj instanceof Uint8ClampedArray) return new Uint8ClampedArray(obj) as Shallowed<T>;
if (obj instanceof Uint16Array) return new Uint16Array(obj) as Shallowed<T>;
if (obj instanceof Uint32Array) return new Uint32Array(obj) as Shallowed<T>;
if (obj instanceof BigUint64Array) return new BigUint64Array(obj) as Shallowed<T>;
if (obj instanceof Int8Array) return new Int8Array(obj) as Shallowed<T>;
if (obj instanceof Int16Array) return new Int16Array(obj) as Shallowed<T>;
if (obj instanceof Int32Array) return new Int32Array(obj) as Shallowed<T>;
if (obj instanceof BigInt64Array) return new BigInt64Array(obj) as Shallowed<T>;
if (obj instanceof Float32Array) return new Float32Array(obj) as Shallowed<T>;
if (obj instanceof Float64Array) return new Float64Array(obj) as Shallowed<T>;
if (obj instanceof ArrayBuffer) return obj.slice(0) as Shallowed<T>;
if (obj instanceof SharedArrayBuffer) return obj.slice(0) as Shallowed<T>;
if (obj instanceof DataView) return new DataView(obj.buffer.slice(0)) as Shallowed<T>;
if (obj instanceof File) return new File([obj], obj.name, { type: obj.type }) as Shallowed<T>;
if (obj instanceof Blob) return new Blob([obj], { type: obj.type }) as Shallowed<T>;

if (typeof obj === 'object') {
return Object.assign({}, obj) as Shallowed<T>;
}
return obj;
return obj as Shallowed<T>;
}

export type Shallowed<T> = Equal<T, ShallowMain<T>> extends true ? T : ShallowMain<T>;
type ShallowMain<T> = T extends [never]
? never
: T extends object
? T extends
| Array<any>
| Set<any>
| Map<any, any>
| Date
| RegExp
| Date
| Uint8Array
| Uint8ClampedArray
| Uint16Array
| Uint32Array
| BigUint64Array
| Int8Array
| Int16Array
| Int32Array
| BigInt64Array
| Float32Array
| Float64Array
| ArrayBuffer
| SharedArrayBuffer
| DataView
| Blob
| File
? T
: OmitNever<{
[P in keyof T]: T[P] extends Function ? never : T[P];
}>
Comment on lines +120 to +122
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what this type does here. Could you explain what this is doing?

I thought cloning some objects with methods, like clone({ foo: () => {} }) will also work. However this type definition does not seem to respect it.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There could be many edge cases, so we might just return T from clone. After all, it's well-known that shallow cloning class instances doesn't copy methods.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A developer working on the code might know if a value has been shallow copied, but other collaborators may not be aware without inspecting the code (specifically, the function returning the cloned value). Therefore, I believe the type proposed by samchon can help reduce misunderstandings among developers.

Using this type enhances code readability and prevents unnecessary bugs and confusion within the team. By employing the Resolved type, we can avoid the misconception of certain methods existing on plain objects, ensuring that developers understand the correct nature of the cloned value and can use the code more accurately.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

스크린샷 2024-07-17 오전 9 01 34 Additionally, as shown in the screenshot, lodash and structuredClone do not support function cloning. It would be clearer for the clone function in es-toolkit to also not support functions

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@raon0211 In the typ level, functional property explicitly removed, otherwise in the variable level, it still alive as a never type. It does not occur compiler level bug because of the characteristics of the never type, but seems ugly. @kakasoo We need to study about it.

image image

Copy link
Collaborator

@raon0211 raon0211 Jul 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand that clone does not clone class instance's methods.

I was wondering what will happen to clone({ foo: () => {} }); Here, we can correctly clone the foo property. However it will be prohibited by types.

As far as I know, we cannot easily distinguish class methods and function properties, since TypeScript uses structural typing.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand that clone does not clone class instance's methods.

I was wondering what will happen to clone({ foo: () => {} }); Here, we can correctly clone the foo property. However it will be prohibited by types.

As far as I know, we cannot easily distinguish class methods and function properties, since TypeScript uses structural typing.

To fix the problem, changed the implementation code.

return Object.fromEntries(
  Object.entries(obj).filter(([_key, value]) => value !== undefined && typeof value !== "function")
)

Copy link
Collaborator

@raon0211 raon0211 Jul 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think changing the implementation is awkward since most people would expect that foo will be properly cloned. Note that lodash will clone those properties. In JavaScript function properties are cloned when shallow copying an object.

const object = { foo: () => {} };
const cloned = { ...object };

I think types should follow the implementation. Implementation should not follow the types.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my opinion, unless there is a TypeScript type that simply handles this case, we allow this behavior, because I think cloning a class instance is not a major use case.

Please refer to our contributing guide where we value simple implementation, and our functions will have complex implementation to suit every usecase.

: T;

type Equal<X, Y> = X extends Y ? (Y extends X ? true : false) : false;
samchon marked this conversation as resolved.
Show resolved Hide resolved
type OmitNever<T extends object> = Omit<T, SpecialFields<T, never>>;
type Primitive = null | undefined | string | number | boolean | symbol | bigint;
type SpecialFields<Instance extends object, Target> = {
[P in keyof Instance]: Instance[P] extends Target ? P : never;
}[keyof Instance & string];

function isPrimitive(value: unknown): value is Primitive {
return value == null ||
(typeof value !== "object" && typeof value !== "function");
return value == null || (typeof value !== 'object' && typeof value !== 'function');
}
2 changes: 1 addition & 1 deletion tsconfig.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"compilerOptions": {
"lib": ["ESNext"],
"lib": ["DOM", "ESNext"],
"target": "es2016",
"module": "ESNext",
"noEmit": true,
Expand Down