Properties of Easing object can reassignment

[MasatoMakino]

Description

All properties of the TWEEN.Easing object allow reassignment.

Expected Behavior

Easing functions and groups in TWEEN.Easing do not allow reassignment.

Actual Behavior

Easing functions and groups in TWEEN.Easing allow reassignment.

Steps to Reproduce

Open code pen

This is a problem section.

TWEEN.Easing.Quadratic = TWEEN.Easing.Linear;
TWEEN.Easing.Cubic.In = TWEEN.Easing.Bounce.Out;

This code can be run without causing any errors.

Impact

In any scope, Easing can be reassignment. If someone accidentally reassignment easing functions, it will be difficult to identify the problem.

Proposal

Ideally, if we try to reassign to TWEEN.Easing, we will get an error.
Even if no error is thrown, this issue will be resolved if the reassignment is ignored.

• Remove Easing global object
• Remove default export in Easing.ts
• Change each EasingFunctionGroup (e.g. Quadratic, Cubic) to Class.
• Change each EasingFunction to static functions
• Change Easing import in index.ts to import * as Easing from ". /Easing.ts"

With this change, I think it will possible to ignore reassignments without losing compatibility.

Environment

macOS 10.15.7
Google Chrome 90.0.4430.93
tween.js 18.6.4

[trusktr]

What if we apply Object.freeze?

[MasatoMakino]

What if we apply Object.freeze?

With this in view, I did some research on how to implement it.

• Object.freeze makes properties, including functions, unchangeable.
• Object.freeze does not support nested objects. Reference
• Static functions are reassignable. class and static functions do not solve this issue.

There was also a review of whether to include the type definition of EasingFunctionGroup in Easing.ts.
#608 (comment)
I could not find a way to expose EasingFunctionGroup interface to outside while exporting Easing global object as default. If you have that information, please let me know.

Proposal

Based on these results, I modify the proposal.

• Remove Easing global object
• Remove default export in Easing.ts
• Change each EasingFunctionGroup (e.g. Quadratic, Cubic) to frozen object.
• Change Easing import in index.ts to import * as Easing from ". /Easing.ts"

This is a sample of Easing.ts

export type EasingFunction = (amount: number) => number

export type EasingFunctionGroup {
	In: EasingFunction
	Out: EasingFunction
	InOut: EasingFunction
}

export const Linear = Object.freeze({
	None: (amount: number): number => {
		return amount
	},
})

export const Quadratic = Object.freeze(<EasingFunctionGroup>{
	In: (amount: number): number => {
		return amount * amount
	},
	Out: (amount: number): number => {
		return amount * (2 - amount)
	},
	InOut: (amount: number): number => {
		if ((amount *= 2) < 1) {
			return 0.5 * amount * amount
		}

		return -0.5 * (--amount * (amount - 2) - 1)
	},
})

Test

I tested this sample in these environments

• TypeScript + tween.esm.js + tween.d.ts : tsc error
• Chrome + tween.umd.js : Uncaught TypeError: Cannot assign to read only property 'Quadratic' of object
• node.js + tween.cjs.js : No error, but ignored.
• nodeunit + tween.cjs.js + rollup.js + @ts-ignore : I can reassign EasingFunctionGroup, but I cannot be reassigned Easing function

If I did not ignore tsc errors intentionally, I got the best results.

If you find any problems with this proposal, please let me know.
Thanks.

[trusktr]

@MasatoMakino Hello again. sorry for taking so long. Let's apply Object.freeze() to the objects, but without changing the export format (because that is a breaking change). f.e.

const Easing = Object.freeze({
	Linear: Object.freeze({
		None: function (amount: number): number {
			return amount
		},
	},
	// ... etc ...
})

export default Easing; // same export, no breaking change

[trusktr]

Another idea is using as const for type checking only, for example:

TS playground

[trusktr]

By the way also @MasatoMakino, are you interested to join the org to help maintain and expand Tween.js? I sent you an invite.