Notistack: onExited callback key mismatch caused by Material-UI

Created on 21 May 2020  路  8Comments  路  Source: iamhosseindhv/notistack

Expected Behavior

When enqueuing a snackbar providing a key as well as a callback for the onExited event, I expected the key given as the second callback argument when it gets fired.

Current Behavior

The onExited event gets fired, but the key argument is always undefined.

Steps to Reproduce

  1. Enqueue a keyed snackbar with a onExited handler and inspect the key argument as soon as it gets fired:
    js enqueueSnackbar("Some message", { key: "test", variant: "default", onExited: (_, myKey) => { console.info("exited with key=" + JSON.stringify(myKey)); } });
  2. Inspect console output.
  3. Behold the message: exited with key=undefined

You can reproduce / verify the problem by checking out the redux integration example on codesandbox.io.

Context

According to the documentation, I'm trying to integrate notistack with a redux state.

Your Environment

| Tech | Version |
|--------------|--------------------------------|
| Notistack | v0.9.16 |
| React | v16.13.1 |
| Browser | Google Chrome (v81.0.4044.129) |
| Material-UI | v4.9.14 |

Most helpful comment

I'm going to open a PR in mui-core to fix this. Until then I'm keeping this open.

All 8 comments

Hey @Gareon, I found your issue from noticing the same behaviour and it turns out that onExited actually has three arguments.

There are still other things wrong with it, but you can fix any issues you're having with the key by doing onExited: (_, _, myKey) => { }

This is bug caused by latest version of material-ui. meaning if you downgrade MUI core it'll start working:
https://github.com/mui-org/material-ui/issues/21091

Alternatively and temporarily, as @actual-size said, you can access the key as the third argument.

I'm going to open a PR in mui-core to fix this. Until then I'm keeping this open.

If I understand the previous API was:

enqueueSnackbar("Some message", {
  key: "test",
  variant: "default",
  onEntered: (node, appearing, key) => {
    console.info("exited with key=" + JSON.stringify(myKey));
  },
  onExited: (node, key) => {
    console.info("exited with key=" + JSON.stringify(myKey));
  }
});

Would it make sense to turn the API to an object?

enqueueSnackbar("Some message", {
  key: "test",
  variant: "default",
  onEntered: ({ node, appearing, key, variant }) => {
    console.info("exited with key=" + JSON.stringify(myKey));
  },
  onExited: ({ node, key, variant }) => {
    console.info("exited with key=" + JSON.stringify(myKey));
  }
});

@oliviertassinari It'd make sense to pass node, isAppearing and key in one object. But it's a breaking change.

Also did you intend to include variant and possibly other options in the callback or was it accidentally added?

@iamhosseindhv Definitely, I was wondering about the merit of the change for a future major. Regarding variant, I wanted to illustrate the future use cases it could allow to support, not sure this prop is valuable specifically.

I agree @oliviertassinari. We have a similar situation in https://github.com/iamhosseindhv/notistack/pull/259 were people need more options to be passed in the callback. Possibly a good feature to have an eye on.

Closing this as my PR https://github.com/mui-org/material-ui/pull/21158 is now merged.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

Mel0nHead picture Mel0nHead  路  5Comments

VincentLanglet picture VincentLanglet  路  3Comments

nebojsanb picture nebojsanb  路  3Comments

alexisab picture alexisab  路  5Comments

lsansaa picture lsansaa  路  7Comments