Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Package development plan #238

Open
1 of 13 tasks
lindyhopchris opened this issue Apr 22, 2023 · 24 comments
Open
1 of 13 tasks

Package development plan #238

lindyhopchris opened this issue Apr 22, 2023 · 24 comments

Comments

@lindyhopchris
Copy link
Contributor

lindyhopchris commented Apr 22, 2023

Many of you may have noticed I haven't been doing a lot of work on this project recently since v3 (Laravel 10) dropped earlier this year. To explain, open source time for me is limited at the moment - I've moved to a job that doesn't use this package plus I've had a lot going on outside of work. The combination means I haven't really been keeping on top of things!

The good news is from the middle of May 2023 I will have more time available to look at the new features that need adding to this package.

I wanted to share here what my priorities are. I.e. what is in-scope for v4. My hit-list of features are as follows:

It's a long list, so it's possible I might spread this over a few major releases, rather than just doing it all for v4.

It's worth noting that this is a big set of work, and will be breaking. To support Atomic Operations and allow the implementation to be consumed programmatically, I need to decouple the implementation from Laravel's form requests. This will be a big step forward, especially as Atomic Operations is one of the big features lots of us want. But it does mean there will be some refactoring when upgrading an application that uses Laravel JSON:API.

Finally I want to mention OpenAPI documentation. I know a lot of people want this. I do want to add it at some point. However, the above list provides a number of features I need in my production APIs. So for me, they are higher priority - and as the creator of this open source project, the reality is I have to prioritise my time on things that are priorities for my projects.

However, the good news is that if I clear the above list, OpenAPI documentation would most likely be the next highest priority to solve.

@lindyhopchris lindyhopchris pinned this issue Apr 22, 2023
@atapatel
Copy link

atapatel commented May 4, 2023

@lindyhopchris Just one suggestion here. pestphp is really cool tool for writing testcase. Can we use it as a defacto standard for testing?

@glennjacobs
Copy link

@lindyhopchris Regarding API documentation, I think this is the most current option which was forked from my original effort https://github.com/swisnl/openapi-spec-generator.

Would it make sense to review and perhaps bring the package into the organisation so it is more official and can be pushed forward?

@lindyhopchris
Copy link
Contributor Author

@glennjacobs I've not had time to look through that yet, but yeah it's likely to be a good starting point when I get to the documentation

@lindyhopchris
Copy link
Contributor Author

Good news, having started developer for v4, I've added JSON Schema as in-scope for development. Which will be a massive step towards documentation.

@ben221199
Copy link

Does version 4 also have optimalizations over version 3? As mentioned in some other issue (or in the Slack), I had the feeling that my code ran way to long when calling the toResponse method.

image
image

I promised you some benchmarking, so that you have an idea where it is slowing down the process. If I have time, I will make some scripts to generate some benchmark data.

@lindyhopchris
Copy link
Contributor Author

@ben221199 as I mentioned in that thread, toResponse is just way too general to know that any performance problems are in this package. That method triggers the encoding - which ends up calling loads of Laravel Eloquent code, plus any logic you've built into the serialization on the resource fields, and any logic you've put on your Eloquent models as well.

As an example, if you're serializing 100 models to resources, with 15 attributes + relations per model, this code in the Eloquent model will be hit 1,500 times:
https://github.com/illuminate/database/blob/3025a3669248185f4af576759b11e461c9edf5b2/Eloquent/Concerns/HasAttributes.php#L428-L455

I'm not seeing any performance issues in the apps I've written. That's not me saying there aren't performance issues - but I'd need something to be traced to a slow execution within this package itself, rather than code in this package that triggers a huge amount of code that's outside of this package's control.

@ben221199
Copy link

That is why I will try some benchmarking in the near future. I hope I can give some information that hopefully can solve some bottlenecks I experience. In case I don't find anything, I should look to other solutions.

@glennjacobs
Copy link

@lindyhopchris Do you have any idea of timeframe for v4?

@lindyhopchris
Copy link
Contributor Author

@glennjacobs I'm working on it at the moment, it's going to take a good few months to work through everything. I might also do it as several major releases, as that'll probably make upgrading a bit easier - i.e. several upgrade steps to move through major versions, rather than one huge upgrade.

@glennjacobs
Copy link

@glennjacobs I'm working on it at the moment, it's going to take a good few months to work through everything. I might also do it as several major releases, as that'll probably make upgrading a bit easier - i.e. several upgrade steps to move through major versions, rather than one huge upgrade.

Sounds good. The bit I'm most interested in is the JSON schema for generating documentation. I'd be keen to help on that with any beta testing or w.e.

@lindyhopchris
Copy link
Contributor Author

Hmm yeah not sure when I'll hit the JSON schema part.

The first release will definitely be the decoupling from form requests (required to unblock atomic operations plus programatic execution) along with moving validation rules to the schemas i.e. validation rules on field classes. That's a relatively large upgrade, which is why I think I'll do it as a release in its own right, with its own upgrade guide.

Then I'll probably add some non-breaking things from the list above during the v4 timeline.

Then v5 will be the remaining breaking things from the list. JSON Schema will probably drop here, as I suspect I might need a few breaking changes to do it.

I'm hoping to be done by the end of July / early August, but it does all depend on how much time I get as I'm doing this all in my free time i.e. no work time on it.

@glennjacobs
Copy link

@lindyhopchris just wondered how you were getting on?

@lindyhopchris
Copy link
Contributor Author

Yeah good thanks. I've refactored the internal plumbing of the package, which was necessary to decouple from Laravel form requests - paving the way for Atomic Operations and programmatic execution.

At the moment I'm working on moving validation rules to the field, filter and pagination classes. Which will give a much more Nova-style validation implementation. This is part of decoupling from the form requests, plus also is going to be much nicer DX.

I'm now planning to release all of the features at the top of this issue over a number of major releases, rather than all at once. That's to make upgrading in stages easier.

@lindyhopchris lindyhopchris changed the title Version 4 development plan Package development plan Aug 31, 2023
@freestyledork
Copy link

@lindyhopchris while you are busy working on refactoring, I am hoping you will consider expanding the sort abilities to allow sorting a main resource based on relation value. This is supported in the JSON API spec. The easiest example might be when using a Profile relation on a User and the profile has additional info contained in it and you would want sort the user list based on some profile field without loading the whole collection into memory. Please see #205 for more details.

I have dug into the code and see no easy way to implement this without considerable refactoring. Perhaps you have a better idea on how to make it work.

@lindyhopchris
Copy link
Contributor Author

@freestyledork if it's possible to support (i.e. we can do it in Eloquent) then yes, it would be good to add this feature. I've asked a question on #205 - can we move the discussion about it there rather than on this issue? Thanks.

@vrusua
Copy link

vrusua commented Mar 23, 2024

Wondering how things are going for you?

@lindyhopchris
Copy link
Contributor Author

I have recently resumed work on this. Balancing the amount of work on it verses other work, but should now be able to make progress. Planning to allocate time each week for it, so I make steady progress rather than putting it totally down for other work.

@elcapo
Copy link

elcapo commented May 2, 2024

Hi @lindyhopchris, first of all, thanks for this amazing package. Definitely, my favourite Laravel package.

I'm extensively using it for a new open source project I'm planning to launch around October, what means I'll be spending a lot of time on using the Laravel Json Api package. I'll be happy to contribute as much as I can.

@lindyhopchris
Copy link
Contributor Author

Ah thanks for the feedback!

I'm close to an alpha release of the re-write of the package internals - which unlocks a huge amount of the new features that need to be built. What's particularly nice about the alpha is it moves validation rules to the fields, filter etc classes. I wasn't sure how that would turn out, but actually it's looking really nice and a good step forward.

It's a bit difficult to get someone to help with getting that alpha out, because so much has changed and really I just need to push it over the line. There might be scope to help beyond that - particularly with trying the alpha out.

@elcapo
Copy link

elcapo commented May 2, 2024

It's a bit difficult to get someone to help with getting that alpha out, because so much has changed and really I just need to push it over the line. There might be scope to help beyond that - particularly with trying the alpha out.

Nice to hear! I'll be waiting for that alpha release to do some testing.

@lindyhopchris
Copy link
Contributor Author

Update and example of schemas in the new world here:
#39 (comment)

@ben221199
Copy link

@ben221199 as I mentioned in that thread, toResponse is just way too general to know that any performance problems are in this package. That method triggers the encoding - which ends up calling loads of Laravel Eloquent code, plus any logic you've built into the serialization on the resource fields, and any logic you've put on your Eloquent models as well.

A year ago I posted something about performance here. Recently I installed Xdebug, so here is some profiling:
image

And even deeper:
image

The total request is 4 seconds. 25% of that is getting my cached models from Redis and unserializing them. The other 75% is encoding it to JSON. It seems that addResourceToIncluded takes a while, specificly getResourceRepresentation.

image

I will debug further to find out if it is only my implementation, or that it is something in the library.

@lindyhopchris
Copy link
Contributor Author

May be create a separate issue if needed - that's all way too specific and not related to this issue which is about package development?

FYI getResourceRepresentation will be triggering all the code in schemas etc to work out what values are in the JSON. Which is why I can't definitely say it's a problem with this package - because it could be hitting implementation-specific code in your schema, your Eloquent model, etc etc.

@ben221199
Copy link

I will investigate further and will create a seperate issue if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants