Octane compatability

[leon0399]

Subject of the issue

Right now library is not Octane compatible, (e.g. places like [this](src/Providers/LaravelServiceProvider.php#48 are not allowed use $this->app, config(...) function call inside providers should be replaced by $this->app->make('config') or similiar). It also caches authenticated user in JWTGuard, which causes user to be not updated between requests

Your environment

Q	A
Bug?	yes
Framework	Laravel
Framework version	8.73.0
Package version	1.2.0
PHP version	8.0.11

Steps to reproduce

Add Octane, add JWT Auth

Expected behaviour

First authenticated user is saved for all subsequent requests

Actual behaviour

Library should work with Octane without extra hacks

[leon0399]

There is also some discussions here

[mfn]

It also caches authenticated user in JWTGuard, which causes user to be not updated between requests

I already sense semantically breaking change and we should probably target 2.x for any one of this.

I mentioned this in another issue already, I think we should focus 1. on being the best and compatible version for ppl to move over from the fork but hold off major chances there.

[leon0399]

I mentioned this in another issue already, I think we should focus 1. on being the best and compatible version for ppl to move over from the fork but hold off major chances there.

Sure I agree! Fix everything we can and only then start thinking on new features!

[leon0399]

But right now we shall think of it like of bugfix, not feature, so we might need to think about finding some kind of clutch to maybe clear this cache on each request

[mfn]

I've a "but" here: the original library didn't claim Octane compatibility and doing so suddenly for essentially (still) the same code base would bring 1.x into the realm of requiring breaking changes.

With "breaking changes" we should include extending this library, which means:

• constructor args changes
• method args changes
• visibility changes (e.g. protected -> private)
• etc.

[Messhias]

In this case, will be great changes, might it be better just we adapt what we have for <8 versions of PHP, and those major changes move to 2.x with 8+ and leave the 1.x to keep supporting older versions until PHP stop support 7.x.

[mfn]

TBH I don't see the connection between "PHP version support", "laravel version support" and "octane support".

I was just suggesting that, in case we need to change too much for Octane (even internally), we should target not 1.x as we would create more friction with upgrade of the forked library.

Even the constructor change recently is already in that direction and can create friction.

[Messhias]

TBH I don't see the connection between "PHP version support", "laravel version support" and "octane support".

I was just suggesting that in case we need to change too much for Octane (even internally), we should target not 1. x as we would create more friction with the upgrade of the forked library.

Even the constructor change recently is already in that direction and can create friction.

So let's put it on the roadmap for release on 2. x, but we should start taking a not in somewhere of his roadmap (I believe here in GitHub there's something called project it's similar to Asana) because as per our discussions about some stuff we're starting inserting a lot of stuff on the roadmap and might well take the development quite delayed.

In the initial, I and @eschricker were thinking to try to deploy new versions monthly of the library and releases for fixes always when necessary.

Do you believe will be interesting we keep 1 to 5 maximum new features monthly?

[mfn]

(I believe here in GitHub there's something called project it's similar to Asana)

A project is "more" than just what we need here, not sure if there's a benefit.

I think what you're looking for are "milestones": each issue can be assigned to one; for example see the ones I used over at https://github.com/rebing/graphql-laravel/milestone/2?closed=1

In the initial, I and @eschricker were thinking to try to deploy new versions monthly of the library and releases for fixes always when necessary.

Do you believe will be interesting we keep 1 to 5 maximum new features monthly?

I believe in:

• release early, release often
• the release process should be as automated as possible to allow the first point
• there's nothing wrong with immediately pushing patch fixes (x.y.z+1)
• or immediately releasing a minor version with a new feature (x.y+1)

I think only the major version should receive more considerations as, in case we want to do semver, it would be the place for breaking changes.

[Messhias]

I agree with your points and I don't know how the milestones in GitHub work, and how to set up and automate.

I can you have more experience on that and in my opinion, I believe we should on this subject, in that case, consider your ideas and move forward.

[mfn]

I don't know how the milestones in GitHub work

I essence, you

• create a new milestone: https://github.com/PHP-Open-Source-Saver/jwt-auth/milestones/new
• then apply it to respective issues/PRs

This is a manual process, it's unrelated to automation (though I believe githubs API supports something here but not sure what).

The milestone page itself as well as every issue/PR it's applied to shows the green bar in order how many of the linked issues/PRs are already solved.

It's a way to "collect" related thing, as in: related to a certain release, for example.

A github project OTOH is more like a kanban-y overview of things in a columnar way. I think this is mostly useful for full-time teams constantly working on stuff, probably not so much for contribution-in-free time though I think milestones can have a place if well maintained.

[leon0399]

@mfn @Messhias as I stated in another issue, let's move to discussion

[dmason30]

@leon0399 I wanted to see if this could be done without breaking changes, since I want to switch one of our apps to using octane. However, I ended up changing one protected method signature and removing another in the provider so I don't think its possible 😢

If interested you can see changes on my fork here: https://github.com/dmason30/jwt-auth/pull/2/files

[mfn]

The changes on the PR look legit to me (briefly looked; I've also made a lib Octane-compatible some time ago and they make sense).

[dmason30]

@mfn I can submit the PR if you want me to after I have tested it, just seems you guys have a mammoth task on deciphering what was originally planned for 2.0, unless you would consider an early 2.0 to add this and perhaps drop PHP 7.4/Laravel 6.

I also have noticed none of the current unit tests touch the service providers (according to coverage) so whenever a change is made to those the tests won't tell us if we have broken anything.

[mfn]

Thanks, yes, makes sense to include this topic into the discussion over at https://github.com/PHP-Open-Source-Saver/jwt-auth/discussions/76#discussioncomment-2012665

[Messhias]

Thanks, yes, makes sense to include this topic into the discussion over at #76 (reply in thread)

And this is will stay using the 2.x versions for PHP >8.0 ou we'll develop in both cases?

[mfn]

@Messhias sorry, I'm not sure what you mean?

[Messhias]

@Messhias sorry, I'm not sure what you mean?

We'll use the branches on 2. x for PHP 8+ or we'll try to use all branches for maximum versions that we can support?

[mfn]

I think this depends on the envisioned time frame but also if it needs a big bang 2.0 branch.

I'm rather for short-lived branches and release early, release often. Also PHP 7.4 still has good security support til Nov 2022 etc.

I think this should be discussed in https://github.com/PHP-Open-Source-Saver/jwt-auth/discussions/76

[mfn]

2.0.0 with Octane support was released!