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.

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

Erroneous "object is not frozen" warnings #7

Open
tdulcet opened this issue Apr 1, 2021 · 3 comments
Open

Erroneous "object is not frozen" warnings #7

tdulcet opened this issue Apr 1, 2021 · 3 comments
Labels
bug Something isn't working

Comments

@tdulcet
Copy link

tdulcet commented Apr 1, 2021

Originally mentioned by @rugk in #6 (comment). Users will get many of these erroneous warnings on extensions with several option groups, like the Awesome Emoji Picker, which is confusing for developers creating PRs (#6 (comment)).

This issue is the unfreezeObject() function:

function unfreezeObject(value) {
// always shallow-copy
if (Array.isArray(value)) {
value = value.slice();
} else if ((isPlainObject(value))) {
value = Object.assign({}, value);
} else {
// return primitive value or similar ones
return value;
}
// but warn if object was not even frozen, so deev can fix the default settings
if (!Object.isFrozen(value)) {
console.warn("The following defined default value of type", typeof value, "is not frozen. It is recommend that all default options are frozen.", value);
}

It checks if the objects are not frozen after doing the shallow-copy. They will of course never be frozen after the shallow-copy, which is why this warning always outputs every time the function is called with an object, regardless of whether the object was initially frozen or not...

@rugk
Copy link
Member

rugk commented Apr 1, 2021

Uuups… thanks a lot!
Indeed seems to be a bug.

Just switching the order of the checks should work, should not it?

If so, you're free to do a PR, so you can get the credit as the commit message author and thanks again for finding and spotting that! 😃

@rugk rugk added the bug Something isn't working label Apr 1, 2021
@tdulcet
Copy link
Author

tdulcet commented Apr 2, 2021

Uuups… thanks a lot!

No problem! I had been meaning to create this for over a year...

Just switching the order of the checks should work, should not it?

Yes, although it may be better to just assign the shallow-copy to a new variable, to avoid the resulting type coercion with non-object arguments.

@rugk
Copy link
Member

rugk commented Apr 2, 2021

Well… the last thing should be no problem, as we use ES2015 or later.
But anyway, also code-style wise it may make sense to rename the variable, so feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants