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

Plugin preferences not persisted with correct key #238

Open
joelamb opened this issue Sep 25, 2023 · 3 comments
Open

Plugin preferences not persisted with correct key #238

joelamb opened this issue Sep 25, 2023 · 3 comments

Comments

@joelamb
Copy link
Contributor

joelamb commented Sep 25, 2023

Plugin preferences should be saved with the key defined in the name property of the class

references:
API documentation
Plugin Interface

The two failing tests in this PR, point to plugin preferences being saved with the Class name (klass.name) as a key, rather than the value of the name property defined on the Class.

This is an issue, because this key is minified in production code, so the preference keys end up being persisted with minified, single-character keys.

This is brittle, since we cannot guarantee the value of the minified Class name, and hence any previously saved user preferences will be lost, if the key changes.

The test output is not particularly helpful in the Checks

But what it's trying to say is:

Expected:  
"plugins": {     "column-reordering": {       "columns": {},       "table": {         "order": {           "A": 3,           "B": 2,           "C": 1,           "D": 0         }       }     },     "column-visibility": {       "columns": {         "A": {},         "B": {},         "C": {},         "D": {}       },       "table": {}     }   } }
-- | --
Result
{   "plugins": {     "ColumnReordering": {       "columns": {},       "table": {         "order": {           "A": 3,           "B": 2,           "C": 1,           "D": 0         }       }     },     "ColumnVisibility": {       "columns": {         "A": {},         "B": {},         "C": {},         "D": {}       },       "table": {}     }   } }

The expected plugin keys are defined with the name property for 'Column Reordering' here

and for 'Column Visibility' here

@joelamb
Copy link
Contributor Author

joelamb commented Sep 25, 2023

@NullVoxPopuli was the intention to move to use class name eg ColumnReordering.name ("ColumnReordering") rather than the name property ("column-reordering")?
If so, how do we avoid having the keys minified in production?

@NullVoxPopuli
Copy link
Contributor

rather than the name property ("column-reordering")?

nay, the manually specified string name was intended to be used exactly to protect against minified class names in production

@joelamb
Copy link
Contributor Author

joelamb commented Sep 26, 2023

Any thoughts on where / how it might have come unstuck, or maybe it was never using the manually-specified name property string?

joelamb added a commit that referenced this issue Sep 27, 2023
use static pluginName not the class name to avoid preferences
being saved with minified ClassName keys see issue #238 for more details
joelamb added a commit that referenced this issue Sep 27, 2023
use static pluginName not the class name to avoid preferences
being saved with minified ClassName keys see issue #238 for more details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants