devour-js/devour-client

The middleware does not exist

gkatsanos opened this issue · 8 comments

api.replaceMiddleware('errors', {
  name: 'errors-with-axios-fix',
  error(payload) {
    const errors = {};
    if (payload.response.data.errors) {
      payload.response.data.errors.forEach((error) => {
        errors[error.source.pointer.split('/').pop()] = error.title;
      });
    }
    return errors;
  },
});

My console errors with
minilog.js:26 devour The middleware errors-with-axios-fix does not exists
and my error handling doesn't do what it's supposed to be doing as per the middleware.

Seems that this broke from "2.0.27", to "2.1.0"

@kevincoenegrachts worked on the PR for this. He'd have more insight.

Looks like we are checking to see if the new middleware name exists in the middleware stack rather than the middleware we would be replacing... There's a debate to be had here about whether this function should keep the middleware it's replacing's name. For backwards compatibility it's necessary that we only check the middleware we're replacing's name.

This:

https://github.com/twg/devour/blob/master/src/index.js#L282-L285

Becomes:

replaceMiddleware (middlewareName, newMiddleware) {
    if (!this.middlewareExists(middlewareName)) {
      Logger.error('The middleware ' + middlewareName + ' does not exists')
      return
    }

    if (this.middlewareExists(newMiddleware.name)) {
      Logger.error('The middleware ' + newMiddleware.name + ' already exists')
      return
    }

    let index = _findIndex(this.middleware, ['name', middlewareName])
    this.middleware[index] = newMiddleware
  }

This should ensure backwards compat.

@gkatsanos In the meantime you can rename your new middleware to "errors" and this should fix the error you're seeing. We will have a fix for this out in the next few hours.

did this got fixed?

@Auspicus Did this get fixed?
I was also wondering where is devour 2.x ? (I saw it mentioned in the README but its nowhere to be found)

This still needs to be fixed. Happy to merge a PR for this if anyone has the time.

tijn commented

@Auspicus I made a PR that will fix this: #206 . Merge at will 😄