Enhance CSSStyleObserver API with Callback Mode as Options#3
Enhance CSSStyleObserver API with Callback Mode as Options#3faizanu94 wants to merge 8 commits intobramus:mainfrom
Conversation
This PR enhances the CSSStyleObserver class by introducing a selective mode. When enabled, this mode ensures that the observer only passes the CSS property that has changed to the callback function, rather than passing all tracked properties
bramus
left a comment
bramus
left a comment
There was a problem hiding this comment.
Thanks for getting started on this. While this PR indeed fulfills the feature request in #1, I think it needs some more discussion to make sure we get the API shape right.
For example, instead of an extra boolean argument added to the constructor, I’d rather have a bag of options with the callbackMode part of that bag. Or maybe even that callbackMode could be part of the CSSStyleObserverCallback – I don’t know.
Furthermore, instead of setting that option to true/false I’d rather prefer an Enum with all the modes. Right now it would be limited to just two (All and Individual) but maybe in the future there could be more modes.
We would also need to decide on the default mode. I think having CallbackModes.Individual as the default would make sense as that’s more performant than populating an response with all observed properties.
|
Thanks for the feedback - I’ve made the changes and opted for option 1 (using an options object for configuration). This approach offers greater flexibility and scalability for the API, as we can easily add more configuration parameters in the future. This design also improves readability by clearly grouping related options I’d personally not opt for making callbackMode a part of the |
README.md
Outdated
| * CallbackMode.INDIVIDUAL: Only the changed property is passed to the callback function | ||
| * CallbackMode.ALL: All observed properties are passed to the callback function, regardless of whether they changed or not | ||
|
|
||
| Try out a demo on CodePen: [https://codepen.io/bramus/pen/WNqKqxj](https://codepen.io/bramus/pen/WNqKqxj?editors=1111) |
There was a problem hiding this comment.
This line is part of the previous section, not the “Callback Modes” section.
src/index.ts
Outdated
| this._callbackMode = options.callbackMode && Object.values(CallbackMode).includes(options.callbackMode) | ||
| ? options.callbackMode | ||
| : CallbackMode.INDIVIDUAL; } |
There was a problem hiding this comment.
I’m no TypeScript expert, but is this check needed here? I would assume TS to validate the options.callbackMode all by itself. If that is the case, then most likely the code can be simplified to this:
this._callbackMode = options.callbackMode ?? CallbackMode.INDIVIDUAL;There was a problem hiding this comment.
I’ve simplified the assignment to this._callbackMode = options.callbackMode ?? CallbackMode.INDIVIDUAL to rely on TypeScript’s type safety. However, I added a fallback in the _handleUpdate method to default to CallbackMode.INDIVIDUAL if an invalid mode is detected at runtime
src/index.ts
Outdated
| .forEach(value => { | ||
| if (this._callbackMode === CallbackMode.INDIVIDUAL && event?.propertyName) { | ||
| const changedProperty = event.propertyName; | ||
| if (this._observedVariables.includes(changedProperty)) { |
There was a problem hiding this comment.
I think this check – that checks if the event.propertyName is part of the observed variables – can apply to both modes.
Suggestion to move the check up a level and have it return early.
Something like this:
- If property is given but the property is not observed: do an early return
- If callback mode is individual and property is given: only get individual property
- If callback mode is all: get all that are observed
There was a problem hiding this comment.
I’ve refactored the code to use a strategy pattern for handling callback modes, moving the modeHandlers outside of the _handleUpdate function (it can have multiple modes in the future). I kept the ?? CallbackMode.INDIVIDUAL in place to ensure that, in non-type-safe environments, any invalid or garbage values for callbackMode still default to a safe, predictable behavior
faizanu94
changed the title
|
This PR has been superseded by newer PRs that add this functionality. Thanks again for your work on this, @faizanu94. |
2 participants
This PR addresses and implements the feature discussed in #1 by enhancing the handling of callback modes in
CSSStyleObserver. A configuration objectoptionshas been introduced andcallbackModeis now included as part of this options object, making the API more flexible and scalableThe implementation now supports two distinct callback modes:
CallbackMode.INDIVIDUAL: Only the changed property is passed to the callback function (default mode)CallbackMode.ALL: All observed properties are passed to the callback function, regardless of whether they changed or notThe mode handling is implemented using a strategy pattern, with each mode’s behavior encapsulated in its own handler function
The README has been updated to reflect these changes, providing usage instructions and descriptions for both callback modes and the default behavior