Fixed issue where Promise rejection was hitting application

[ffxsam]

Please make sure these boxes are checked before submitting your PR, thank you!

• Make sure you follow Element's contributing guide (中文 | English | Español | Français).
• Make sure you are merging your commits to dev branch.
• Add some descriptions and refer relative issues for you PR.

The uncaught rejection was bubbling up our application. And since we're using an error logger like Sentry, it was catching those and logging them. The error isn't really an error at all, so it needs to be caught and ignored in this case.

[element-bot]

Deploy preview for element ready!

Built with commit 8a2dbed

https://deploy-preview-14816--element.netlify.com

[wacky6]

What cause the rejection.

Silently ignoring errors is a troublesome practice. Please find the source of error.

[ffxsam]

What cause the rejection.

Silently ignoring errors is a troublesome practice. Please find the source of error.

What do you mean? This component is causing the rejection, intentionally. before-leave is doing it, specifically. To quote the docs:

hook function before switching tab. If false is returned or a Promise is returned and then is rejected, switching will be prevented

In my case, it's an async process and so I have no choice but to return a rejected Promise. This component needs to catch it and ignore it, because it's not a real error. Using Promises in this way is anti-pattern, it would've been better to return a resolved Promise with either a true or false value.

[wacky6]

Use, and keep comments in catchFn

before.then(_ => {...}, _ => {})

before.then.catch will ignore error in .then, which should not be ignored.

[ffxsam]

The Promise rejection in this case is only used to determine whether the tab should change. I don't see the problem with catching it to ignore it.

Here's a live example: https://jsfiddle.net/q9tdm3r6/

Try to switch tabs, then look at the console. You can see the unhandled Promise rejection. beforeLeave method must be async. How would you change this code so that the Promise rejection isn't unhandled?

My point is, it should be possible to either return false from a regular function, or false from an async function (i.e. Promise.resolve(false)) to prevent tab switch. Using a rejected Promise is not ideal.

[wacky6]

I know it is not ideal. But we can't change API unless it's a major version.

.then.catch will catch and ignore error in changeCurrentName and $refs.nav.removeFocus. Their error should not be ignored because they are part of framework, not user code. See: https://jsfiddle.net/3wsLrqmv/

[ffxsam]

I understand the idea of catching to ignore an error. Let's look at this specific example:

<el-tabs :before-leave="beforeLeave">

methods: {
  async beforeLeave(activeName) {
    if (activeName !== '0') throw new Error('prevent tab change');
  }
}

In this example, how would you prevent the tab change without resulting in an uncaught Promise rejection? I have to throw an error in order to prevent the tab change, it cannot be caught/ignored in my own code and still have the desired effect. At least, I haven't figured out a way to do this so far.

[wacky6]

I mean write your PR like this:

if (before && before.then) {
  before.then(
    () => {
      changeCurrentName();
      this.$refs.nav && this.$refs.nav.removeFocus();
    },
    _ => {
      // https://github.com/ElemeFE/element/pull/14816/
      // ignore promise rejection in `before-leave` hook
    }
  );
}

[ffxsam]

Ahh, sorry, I misunderstood. Will update in a bit!

[ffxsam]

@wacky6 Updated. Good call on removing the catch()