[Autocomplete] Tom Select Options Plugins #2657
Conversation
luchidalgo
commented
Mar 25, 2025
luchidalgo
commented
| Q | A |
|---|---|
| Bug fix? | yes |
| New feature? | no |
| Docs? | no |
| Issues | Fix #2656 |
| License | MIT |
carsonbot
added
Autocomplete
Bug
📊 Packages dist files size differenceThanks for the PR! Here is the difference in size of the packages dist files between the base branch and the PR.
|
|||||||||
| if ('plugins' in this.tomSelectOptionsValue && Array.isArray(this.tomSelectOptionsValue.plugins)) { | ||
| this.tomSelectOptionsValue.plugins.forEach((pluginName) => { | ||
| plugins[pluginName] = {}; | ||
| }); | ||
| } |
There was a problem hiding this comment.
Given examples (https://tom-select.js.org/plugins/caret-position/ and https://tom-select.js.org/plugins/checkbox-options/) it looks like plugins can also be a key-value object.
Can't we use a more simple solution like this?
| if ('plugins' in this.tomSelectOptionsValue && Array.isArray(this.tomSelectOptionsValue.plugins)) { | |
| this.tomSelectOptionsValue.plugins.forEach((pluginName) => { | |
| plugins[pluginName] = {}; | |
| }); | |
| } | |
| plugins = this.tomSelectOptionsValue.plugins || []; |
There was a problem hiding this comment.
This one is treaky : https://tom-select.js.org/docs/plugins/ says we can put just plugin names (simple array) or plugin with options (object in js side). Also with simple assignment we override plugins configuration added line 138 to 150
| @@ -214,7 +220,7 @@ export default class extends Controller { | |||
| config.shouldLoad = () => false; | |||
| } | |||
|
|
|||
| return this.#mergeObjects(config, this.tomSelectOptionsValue); | |||
| return this.#mergeObjects(this.tomSelectOptionsValue, config); | |||
There was a problem hiding this comment.
I'm not really sure about that, it looks like a BC no?
There was a problem hiding this comment.
By doing this, we prevent user-provided options from overwriting the configuration created within this function, especially for plugins.
There was a problem hiding this comment.
As @Kocal said, it is BC break. By looking at the code, the original idea is: if a user provides some config values, these values always win over default config
There was a problem hiding this comment.
Maybe I can revert this change and merge the config with a copy of tomSelectOptionsValue, without the plugins previously merged by a specific process, in order to avoid them being replaced ?
carsonbot
added
Status: Needs Work
