Code transformed incorrectly

[z4o4z]

Rollup Version

3.21.0

Operating System (or Browser)

MacOS Version 13.3.1 (22E261)

Node Version (if applicable)

No response

Link To Reproduction

short example

I use library that has pretty weird code, it passes callbacks object into react components and other functions, and these components/functions mutate callbacks object. Other functions uses callbacks object to perform some logic, but rollup drops callbacks from them. It introduces really weird issues in our product. The example link is not perfect one (it doesn't use the props object as an argument), but it shows exactly what happens in my production bundle . If remove initial nullable values from the callbacks object, all works fine.

Expected Behaviour

 keyBindingFn: function keyBindingFn(keyboardEvent) {
      if (callbacks.keyBindingFn) return callbacks.keyBindingFn(keyboardEvent);
      return undefined;
    },
    handleReturn: function handleReturn(keyboardEvent) {
      if (callbacks.handleReturn) return callbacks.handleReturn(keyboardEvent);
      return undefined;
    },
    handlePastedText: function handlePastedText(keyboardEvent) {
      if (callbacks.handlePastedText) return callbacks.handlePastedText(keyboardEvent);
      return undefined;
    },

Actual Behaviour

  keyBindingFn: function keyBindingFn(keyboardEvent) {
      return undefined;
    },
    handleReturn: function handleReturn(keyboardEvent) {
      return undefined;
    },
    handlePastedText: function handlePastedText(keyboardEvent) {
      return undefined;
    },

[dnalborczyk]

just glancing over the repro, the outcome appears to be correct.

callbacks.keyBindingFn is undefined, therefore if (callbacks.keyBindingFn) return callbacks.keyBindingFn(keyboardEvent); is being removed.

[lukastaegert]

Yes, and since you never call modify with a parameter, it is impossible for callbacks to be changed.

[z4o4z]

yeah, but the original code is much bigger and more complicated. The modify function is called somewhere outside of this module... I can't reproduce this issue in the reply, but it constantly reproduced in the app. As a workaround I've forked a lib and removed the initial undefined values, but it's not ideal :(

[z4o4z]

here's the library bundled code, as you can see callbaks are used in the DecoratedMentionSuggestionsComponent as props and later in the return object methods

.

But in the application bundle it looks like this (I've removed some unrelated code to make it easier to read):

As you can see, it removed the function's call and just returns the function itself. This causes a huge issue...

I'm using vite to bundle my app. The minification is disabled in my tests, so I assume it's a rollup issue. Probably I'm wrong

[lukastaegert]

In your original code, where is the place that updates callbacks?

[z4o4z]

have no idea, i'm using draftjs editor with plugins and one of these plugins mutate callbacks. In runtime, i see that callbacks are mutated, functions are there

[dnalborczyk]

@lukastaegert from the first screenshot, callbacks are attached to an object mentionSearchProps

which is being passed on to React.createElement, with the result of _extends:

which might either get merged/extended by the passed in props (_extends(...)) or could get mutated afterwards somewhere.

[z4o4z]

@lukastaegert from the first screenshot, callbacks are attached to an object mentionSearchProps

which is being passed on to React.createElement, with the result of _extends:

which might either get merged/extended by the passed in props (_extends(...)) or could get mutated afterwards somewhere.

exactly, i can add a proxy to find a place where callbacks are mutated, if we really need it

[lukastaegert]

Ah, thanks, I overlooked that. I fear we will need a better reproduction to dig into this. I don’t suppose you have a repository to share? Would also be interesting if Rollup 3.18.0 already had this issue or if 3.19.0 introduced it. It looks like _extends does not deoptimize its argument.

[lukastaegert]

What is the code of the _extends function in this example?

[z4o4z]

_extends is pretty simple, just a polyfill for Object.assign:

function _extends() {
  _extends = Object.assign || function (target) {
    for (var i = 1; i < arguments.length; i++) {
      var source = arguments[i];

      for (var key in source) {
        if (Object.prototype.hasOwnProperty.call(source, key)) {
          target[key] = source[key];
        }
      }
    }

    return target;
  };

  return _extends.apply(this, arguments);
}

[dnalborczyk]

could you create a repository with a minimal repro @z4o4z ? otherwise it's nearly impossible to tell what's going on.

[lukastaegert]

That may not be needed, I think I figured it out. It is the use of arguments in _extends that breaks it, see this example.

This was not a problem prior to version 3.19.0 because here, call arguments were deoptimized by default, see here.

I will see what I can do.

[lukastaegert]

Potential fix at #4965

[rollup-bot]

This issue has been resolved via #4965 as part of rollup@3.21.1. You can test it via npm install rollup.