fix: minor stuff

[vonagam]

Minor stuff:

1. Moved @feathers/* dependencies from devDependencies to dependencies if their types are present in a package build.

2. Moved this line up, since no need to call getPayload and getTokenOptions if their result is irrelevant.

3. Here used location as is, without prepending slash, as it will be stripped anyway.

4. In @feathersjs/configuration allow calling without an argument and use app!. instead of (app as Application)..

5. No need to service && here since line before will throw if service is undefined, also in types service is non-nullable.

Note:

6. @feathersjs/commons seems to be stuck at 4.3.0 while everyone else is already at 4.3.2 (it skipped 4.3.1 too).

[daffl]

Sweet, all makes sense, thank you! I was actually wondering why all other dependencies always get updated but it's basically when a dependent package has a new version and @feathersjs/commons doesn't depend on anything so it doesn't need to get updated.

[vonagam]

Ah... i get it now. It is because of this file. Because almost everything depends on @feathersjs/feathers when you update this file - and you do it every new version regardless if @feathersjs/feathers had any changes - almost everything gets new version. Except @feathersjs/commons because it does not depend on any feathers packages.

[bertho-zero]

In my opinion it should rather be peerDependencies to favor the version installed by the developer and avoid duplicates.

[jalbersdorfer]

Hi, your 2nd change breaks my implementation.
I use it to concretize a jwt authentication as following:

I login to my application using Username + Password (LocalStrategy).
This Strategy loads a list of Tenants the logged in user has access to.
Then the User can choose a specific tenant.
Now I use the JWT Token I got from the LocalStrategy to login via JWTStrategy and do a kind of a re-authentication to obtain a new token.

And there's the point. In getPayload, I check whether I have to recreate the token and delete the old one from authResult if so. Which causes in turn that a new token with new Payload gets created.

With the current (your) Implementation, I have no clue how I could achieve the same, since getPayload will never again get called for JWTStrategy or any derived.

[daffl]

I think it is valid to change this back if you can add a test that illustrates your use case.

[vonagam]

You can also extend default AuthenticationService with functionality that you want. For example, if i understand correctly, your case can be achieved by:

class MyAuthenticationService extends AuthenticationService {
  async create (data, params) {
    const authResult = await super.create(data, params);
    // do your checks of authResult here
    return authResult;
  }
}

Or even hooks may work (and be more feathers-way) since create is a standard feathers method and authResult will be in context.result in after hooks.

[jalbersdorfer]

Thanks guys for your quick response and thank you @vonagam for your hint about overriding the create method in my own AuthenticationService. I already had a derivate of it in order to extend getPayload. I definitively get your point from the performance optimization standpoint, but the outcome for my scenario is all but nice nor clean.

So I want step back a little and thank @daffl for your support about moving back. @vonagam, look at it in a way of consistency. Until now, for every AuthenticationStrategy getPayload got called. Now only for Strategies not authenticating themselfe with a token getPayload gets called.

And from this perspective I also would appreciate to change this back to the consistent behavior.
What are your thoughts about this?

[vonagam]

Why are you against overriding create (or using hook which is better if it is possible) for your use-case while ok with overriding getPayload?

[jalbersdorfer]

because that's exactly what I do - I want to modify/change the Payload.

[vonagam]

One decides whenever to use old token or create new one in authenticate (by returning authResult with or without accessToken). So in your case you may want to consider custom auth strategy as an appropriate solution.

For me getPayload simply returns a payload for a newly created token and the method should not be expected to modify authResult or params. Especially since it is called in parallel with getTokenOptions and if you are ok with both of them modifying their arguments you will end up with race conditions...

[jalbersdorfer]

Yes, you are right! - It Works! Thanks!
It is perfectly clean and nice. It is not prone to race condition, either. Just a perfect solution.
I was, for some reason fixed to the idea of having to overwrite getPayload to modify the payload.