hybridsjs / hybrids

Extraordinary JavaScript UI framework with unique declarative and functional architecture
https://hybrids.js.org
MIT License
3.03k stars 85 forks source link

Protect Models from incorrect use #270

Closed Qsppl closed 1 week ago

Qsppl commented 1 week ago

Developers who are just starting to work with hybrids.js often complain that it is difficult for them to understand where they made a mistake and what it was.

I have collected various use cases for Store Definition:

https://codepen.io/qsppl/pen/poXxQqd

Use cases that generate an error are commented out so that the example can be run

import { store } from "https://esm.sh/hybrids"

// see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects

const StoreDefinition = {
    // <<< TypeError: Cannot freeze
    // case01: globalThis,
    // <<< TypeError: Cannot freeze
    // case01_list: [globalThis],

    case02: Infinity,
    case02_list: [Infinity],

    case03: NaN,
    case03_list: [NaN],

    // <<< TypeError: The value of the 'case04' must be a string, number or boolean: undefined
    // case04: undefined,
    // <<< TypeError: The value of the 'case04_list' must be a string, number or boolean: undefined
    // case04_list: [undefined],

    // <<< TypeError: The value of the 'case05' must be a string, number or boolean: null
    // case05: null,
    // <<< TypeError: Model definition must be an object: object - hahaha
    // case05_list: [null],

    case06_constructor: Object,
    // <<< TypeError: The array item for the 'case06_constructor_list' must be one of the primitive types constructor: String, Number, or Boolean
    // case06_constructor_list: [Object],
    case06_instance: new Object(),
    case06_instance_list: [new Object()],
    case06_literal: {},
    case06_literal_list: [{}],

    case07_constructor: Function,
    // <<< TypeError: The array item for the 'case07_constructor_list' must be one of the primitive types constructor: String, Number, or Boolean
    // case07_constructor_list: [Function],
    case07_instance: new Function(),
    // <<< TypeError: The array item for the 'case07_instance_list' must be one of the primitive types constructor: String, Number, or Boolean
    // case07_instance_list: [new Function()],
    case07_literal: () => {},
    // <<< TypeError: The array item for the 'case07_literal_list' must be one of the primitive types constructor: String, Number, or Boolean
    // case07_literal_list: [() => {}],

    case08_constructor: Boolean,
    case08_constructor_list: [Boolean],
    // <<< TypeError: You must use primitive boolean value for 'case08_instance' case of the provided model definition
    // case08_instance: new Boolean(),
    case08_instance_list: [new Boolean()],
    case08_literal: false,
    case08_literal_list: [false],

    case09_constructor: Symbol,
    // <<< TypeError: The array item for the 'case09_constructor_list' must be one of the primitive types constructor: String, Number, or Boolean
    // case09_constructor_list: [Symbol],
    // <<< TypeError: The value of the 'case09_instance' must be a string, number or boolean: symbol
    // case09_instance: Symbol(),
    // <<< TypeError: The value of the 'case09_instance_list' must be a string, number or boolean: symbol
    // case09_instance_list: [Symbol()],

    // () => <<< Error: undefined
    case10_constructor: Error,
    // <<< TypeError: The array item for the 'case10_constructor_list' must be one of the primitive types constructor: String, Number, or Boolean
    // case10_constructor_list: [Error],
    case10_instance: new Error(),
    case10_instance_list: [new Error()],

    case11_constructor: Number,
    case11_constructor_list: [Number],
    // <<< TypeError: You must use primitive number value for 'case11_instance' case of the provided model definition
    // case11_instance: new Number("3"),
    case11_instance_list: [new Number("3")],
    case11_literal: 3,
    case11_literal_list: [3],

    // () => <<< Exception: TypeError: Cannot convert undefined to a BigInt
    case12_constructor: BigInt,
    // <<< TypeError: The array item for the 'case12_constructor_list' must be one of the primitive types constructor: String, Number, or Boolean
    // case12_constructor_list: [BigInt],
    // <<< TypeError: The value of the 'case12_instance' must be a string, number or boolean: bigint
    // case12_instance: BigInt(""),
    // <<< TypeError: The value of the 'case12_instance_list' must be a string, number or boolean: bigint
    // case12_instance_list: [BigInt("")],

    case13: Math,
    // <<< TypeError: Nested model definition cannot be used outside of the parent definition
    // case13_list: [Math],

    case14_constructor: Date,
    // <<< TypeError: The array item for the 'case14_constructor_list' must be one of the primitive types constructor: String, Number, or Boolean
    // case14_constructor_list: [Date],
    case14_instance: new Date("0"),
    case14_instance_list: [new Date("0")],

    case15_constructor: String,
    case15_constructor_list: [String],
    // <<< TypeError: You must use primitive string value for 'case15_instance' case of the provided model definition
    // case15_instance: new String(),
    case15_instance_list: [new String()],
    case15_literal: "",
    case15_literal_list: [""],

    case16_constructor: RegExp,
    // <<< TypeError: The array item for the 'case16_constructor_list' must be one of the primitive types constructor: String, Number, or Boolean
    // case16_constructor_list: [RegExp],
    case16_instance: new RegExp(""),
    case16_instance_list: [new RegExp("")],
    case16_literal: /.*/,
    case16_literal_list: [/.*/],

    case17_constructor: Array,
    // <<< TypeError: The array item for the 'case17_constructor_list' must be one of the primitive types constructor: String, Number, or Boolean
    // case17_constructor_list: [Array],
    // <<< TypeError: The value of the 'case17_instance' must be a string, number or boolean: undefined
    // case17_instance: new Array(),
    case17_instance_list: [new Array()],
    // <<< TypeError: The value of the 'case17_literal' must be a string, number or boolean: undefined
    // case17_literal: [],
    case17_literal_list: [[]],

    // () => <<< Exception: TypeError: Constructor Int8Array requires 'new' at Int8Array
    case18_constructor: Int8Array,
    // <<< TypeError: The array item for the 'case18_constructor_list' must be one of the primitive types constructor: String, Number, or Boolean
    // case18_constructor_list: [Int8Array],
    case18_instance: new Int8Array(),
    case18_instance_list: [new Int8Array()],

    // () => <<< Exception: TypeError: Constructor Map requires 'new' at Map
    case19_constructor: Map,
    // <<< TypeError: The array item for the 'case19_constructor_list' must be one of the primitive types constructor: String, Number, or Boolean
    // case19_constructor_list: [Map],
    case19_instance: new Map(),
    case19_instance_list: [new Map()],

    // () => <<< Exception: TypeError: Constructor Set requires 'new' at Set
    case20_constructor: Set,
    // <<< TypeError: The array item for the 'case20_constructor_list' must be one of the primitive types constructor: String, Number, or Boolean
    // case20_constructor_list: [Set],
    case20_instance: new Set(),
    case20_instance_list: [new Set()],

    // () => <<< Exception: TypeError: Constructor WeakMap requires 'new' at WeakMap
    case21_constructor: WeakMap,
    // <<< TypeError: The array item for the 'case21_constructor_list' must be one of the primitive types constructor: String, Number, or Boolean
    // case21_constructor_list: [WeakMap],
    case21_instance: new WeakMap(),
    case21_instance_list: [new WeakMap()],

    // () => <<< Exception: TypeError: Constructor WeakSet requires 'new' at WeakSet
    case22_constructor: WeakSet,
    // <<< TypeError: The array item for the 'case22_constructor_list' must be one of the primitive types constructor: String, Number, or Boolean
    // case22_constructor_list: [WeakSet],
    case22_instance: new WeakSet(),
    case22_instance_list: [new WeakSet()],

    // () => <<< Exception: TypeError: Constructor ArrayBuffer requires 'new' at ArrayBuffer
    case23_constructor: ArrayBuffer,
    // <<< TypeError: The array item for the 'case23_constructor_list' must be one of the primitive types constructor: String, Number, or Boolean
    // case23_constructor_list: [ArrayBuffer],
    case23_instance: new ArrayBuffer(1),
    case23_instance_list: [new ArrayBuffer(1)],

    // () => <<< Exception: TypeError: Constructor DataView requires 'new' at DataView
    case25_constructor: DataView,
    // <<< TypeError: The array item for the 'case25_constructor_list' must be one of the primitive types constructor: String, Number, or Boolean
    // case25_constructor_list: [DataView],
    case25_instance: new DataView(new ArrayBuffer(1)),
    case25_instance_list: [new DataView(new ArrayBuffer(1))],

    case26: Atomics,
    // <<< TypeError: Nested model definition cannot be used outside of the parent definition
    // case26_list: [Atomics],

    case27: JSON,
    // <<< TypeError: Nested model definition cannot be used outside of the parent definition
    // case27_list: [JSON],

    // () => <<< Exception: TypeError: Promise constructor cannot be invoked without 'new' at Promise
    case28_constructor: Promise,
    // <<< TypeError: The array item for the 'case28_constructor_list' must be one of the primitive types constructor: String, Number, or Boolean
    // case28_constructor_list: [Promise],
    case28_instance: new Promise(() => {}),
    // <<< TypeError: Nested model definition cannot be used outside of the parent definition
    // case28_instance_list: [new Promise(() => {})],

    case29: Reflect,
    // <<< TypeError: Nested model definition cannot be used outside of the parent definition
    // case29_list: [Reflect],

    // () => <<< Exception: TypeError: Constructor Proxy requires 'new'
    case30_constructor: Proxy,
    // <<< TypeError: The array item for the 'case30_constructor_list' must be one of the primitive types constructor: String, Number, or Boolean
    // case30_constructor_list: [Proxy],
    case30_instance: new Proxy({}, {}),
    case30_instance_list: [new Proxy({}, {})],

    // <<< TypeError: Nested model definition cannot be used outside of the parent definition
    // case31: Intl,
    case31_list: [Intl],
}

console.log(store.get(StoreDefinition))

There are indeed many cases when, if the library is used incorrectly, it generates an ambiguous error message or does not generate an error at all.

For example, this very common formulation of the error does not tell the truth: The value of the 'case04' must be a string, number or boolean: undefined. In fact, a property can be not only a string, number or boolean, but also an object or an array or a getter or a record.

Or in case 17 the error now appears: TypeError: The value of the 'case17_literal' must be a string, number or boolean: undefined. But a much better fit would be an error like TypeError: The array item for the 'case17_literal' must be one of the primitive types constructor: ..., ... or ...

Can you make more user-friendly errors so that newbies don't get confused about what they're doing?

P.S. Let me clarify that junior developers also use the library, so you shouldn’t ignore even the most idiotic cases of using the library, for example:

const StoreDefinition = {
    // <<< TypeError: Cannot freeze
    case01: globalThis,
}
fruitbang commented 1 week ago

case17_literal: [] is the most important, and most confusing case, imho, so i want to see somethin like this:

"Uncaught TypeError: The value of the [] must be a string, number, or boolean.

This error occurs when you try to assign an undefined value to a property or variable that expects a specific data type. For example: const Model = { // Array anyArr: [], }

To fix this error, you need to make sure that you are assigning a valid value to the property or variable. Here are a few >examples of how you can assign valid values: const Model = { // Primitive permissions: ["user", "admin"], // Nested objects images: [ { url: "https://example.com/large.png", size: "large" }, { url: "https://example.com/medium.png", size: "medium" }, ], }

For more information on working with nested objects and models in Hybrids.js, please refer to the official documentation: https://hybrids.js.org/#/store/model?id=nested-object

fruitbang commented 1 week ago

image

smalluban commented 1 week ago

The error message is correct - you cannot crate a model with an empty array - it must have a default value inside. Please try to do the same without store.record() - the error is the same.

The error message could improve, as it takes the value (undefined), so it's primitive, but it's not a string, boolean or number. However, you can pass there an object too.

fruitbang commented 1 week ago

The error message is correct - you cannot crate a model with an empty array - it must have a default value inside. Please try to do the same without store.record() - the error is the same.

The error message could improve, as it takes the value (undefined), so it's primitive, but it's not a string, boolean or number. However, you can pass there an object too.

When i try to use [] or {} as value of Model, and see the following error: "Uncaught TypeError: The value of the 'value' must be a string, number or boolean: undefined", I see only one thing, - that in models you can't use JS objects (objects, arrays etc...) , but only primitives... Text of the error message is so confusing, that 3 out of 4 developers who tried to use hybrids in my circle (with me) were sure that you can't use objects and arrays, but only other Models for nested data

https://github.com/hybridsjs/hybrids/issues/270#issuecomment-2326704389

fruitbang commented 1 week ago

image image

there is a couple of variant of more friendly messages, in which the developers tried to predict the probable cause of this error and tried to immediately help, rather than confuse the user who made this error.

smalluban commented 1 week ago

Using an empty object as a property value makes no sense, as it would always be empty. You have to fill it with something...

Here you have docs for nested objects: https://hybrids.js.org/#/store/model?id=nested-object

fruitbang commented 1 week ago

Nice text for error message, bro)

Using an empty object as a property value makes no sense, as it would always be empty. You have to fill it with something...

Here you have docs for nested objects: https://hybrids.js.org/#/store/model?id=nested-object

smalluban commented 1 week ago

I agree, I'll look into it. It should be a simple change. As an author I rarely make those mistakes.

smalluban commented 1 week ago

I added two new error messages for empty array and empty nested object. However, I have no intention to "protect" the store from passing built-in objects or other incorrect values.

If a user of the framework would read the docs first, there won't be a problem - as it defines what you can or cannot pass.

The library throws to protect execution from being broken more widely, it's not another layer of the documentation like I try to pass something very stupid and see if the library will show me what I did wrong.

I would never thought, that I could define my model like value: Math... why does anyone would ever think about it?

Feel free to re-open, if you have any other "real world" scenarios - where after reading documentation it's still not obvious how to use it.