Skip to content

react-flatpickr mergeHooks onChange accumulation #263

Description

@altafmubarak

Bug: mergeHooks() accumulates onChange handlers by mutating the caller-provided options object

react-flatpickr version: 4.0.11
Flatpickr version: 4.6.13
Environment: React application using the options object together with the onChange prop

Description

mergeHooks() currently mutates the options object passed by the consumer when merging hook props such as onChange.

This can cause the same prop callback to be appended to the existing hook array on every parent render.

The issue is reproducible even when both the options object and the onChange callback have stable references.

Reproduction

CodeSandbox:

https://codesandbox.io/p/sandbox/6z5xhf?file=%2Fsrc%2FTestMergeHooks.js%3A98%2C2

The reproduction uses a memoized options object and a memoized onChange callback:

import React, {
  useMemo,
  useState,
  useCallback,
  useEffect,
} from "react";

import Flatpickr from "react-flatpickr";

import "flatpickr/dist/themes/material_blue.css";

export default function TestMergeHooks() {
  const [count, setCount] = useState(0);

  // Same options object across all renders.
  const options = useMemo(() => {
    return {
      enableTime: true,
      allowInput: true,

      onChange: (dates, value) => {
        console.log("CONFIG HANDLER FIRED", {
          value,
        });
      },
    };
  }, []);

  // Same callback reference across all renders.
  const handleChange = useCallback(
    (dates, value) => {
      console.log("PROP HANDLER FIRED", {
        value,
      });
    },
    []
  );

  useEffect(() => {
    console.log("================================");
    console.log("AFTER RENDER", count);
    console.log("OPTIONS OBJECT:", options);

    if (Array.isArray(options.onChange)) {
      console.log(
        "onChange ARRAY LENGTH:",
        options.onChange.length
      );

      console.log(
        "onChange HANDLERS:",
        options.onChange
      );

      console.log(
        "SAME PROP HANDLER REFERENCES:",
        options.onChange.map(
          (fn) => fn === handleChange
        )
      );
    }

    console.log("================================");
  });

  return (
    <div style={{ padding: 20 }}>
      <button
        onClick={() => setCount((c) => c + 1)}
      >
        Re-render Parent ({count})
      </button>

      <br />
      <br />

      <Flatpickr
        options={options}
        onChange={handleChange}
      />
    </div>
  );
}

Actual result

After the initial render, the onChange array contains two handlers:

onChange ARRAY LENGTH: 2
SAME PROP HANDLER REFERENCES: [false, true]

After one parent re-render:

onChange ARRAY LENGTH: 3
SAME PROP HANDLER REFERENCES: [false, true, true]

After two parent re-renders:

onChange ARRAY LENGTH: 4
SAME PROP HANDLER REFERENCES: [false, true, true, true]

After three parent re-renders:

onChange ARRAY LENGTH: 5
SAME PROP HANDLER REFERENCES: [false, true, true, true, true]

The important point is that handleChange is memoized with useCallback([]), so the same function reference is being appended repeatedly.

When a date is selected after three re-renders, the handlers are executed as:

CONFIG HANDLER FIRED
PROP HANDLER FIRED
PROP HANDLER FIRED
PROP HANDLER FIRED
PROP HANDLER FIRED

Why this happens

The current merge logic modifies the supplied inputOptions object directly:

const existingHookFn = inputOptions[hook];

...

if (existingHookFn && !Array.isArray(existingHookFn)) {
  inputOptions[hook] = [inputOptions[hook]];
}

...

inputOptions[hook].push(...propHook);

On the first render:

options.onChange = [configHandler, handleChange]

On the next render, the same options object is reused. Because options.onChange is already an array, the existing array is preserved and the prop handler is pushed again:

[configHandler, handleChange]
        ↓
[configHandler, handleChange, handleChange]
        ↓
[configHandler, handleChange, handleChange, handleChange]
        ↓
[configHandler, handleChange, handleChange, handleChange, handleChange]

This means a caller-owned options object becomes progressively mutated by the wrapper.

Expected result

The original options object should not be mutated.

Repeated parent renders should not accumulate previously merged prop handlers.

For example, conceptually:

Original options:

{
  onChange: configHandler
}

should remain unchanged.

The wrapper should derive a new merged object for each render:

const nextOptions = {
  ...inputOptions,
};

and merge the current prop hooks into that new object.

The expected onChange configuration should remain equivalent to:

[
  configHandler,
  handleChange
]

regardless of how many times the parent component re-renders.

Suggested fix

Avoid mutating inputOptions inside mergeHooks().

For example:

const mergeHooks = (
  inputOptions,
  props
) => {
  const nextOptions = {
    ...inputOptions,
  };

  hooks.forEach((hook) => {
    const existingHook = inputOptions[hook];
    const propHook = props[hook];

    if (!propHook) {
      return;
    }

    const existingHooks = existingHook
      ? Array.isArray(existingHook)
        ? existingHook
        : [existingHook]
      : [];

    const propHooks = Array.isArray(propHook)
      ? propHook
      : [propHook];

    nextOptions[hook] = [
      ...existingHooks,
      ...propHooks,
    ];
  });

  return nextOptions;
};

The key requirement is that mergeHooks() should not modify the caller-provided inputOptions object.

Additional note

A related issue already exists regarding onChange behavior between multiple Flatpickr components:

This reproduction appears to be a different issue: the problem occurs within a single component because the same onChange handler is accumulated in the options object across renders.

Impact

Applications that reuse a stable options object while the wrapper performs repeated prop merging can accumulate duplicate hook references.

As a result, one logical date change can invoke the same application callback multiple times.

This can lead to duplicated state updates, duplicated side effects, or other unexpected behavior.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions