-
Notifications
You must be signed in to change notification settings - Fork 108
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
Future design and request for comments #411
Comments
First of all, thank you @lindyhopchris for the huge effort that you are putting into developing and maintaining this package. As for the feedback, I like most of what you've written and the goals that you wish to achieve with v2. The idea that neomerx should just be an implementation detail is great 👍 The only thing that I dislike in this approach is that the controller is no longer optional. The current way with a default controller and the convention of adapter/schema/validator/authorizer/content negotiator felt very robust and modular and I've never had to create my custom controller. In this new approach this part (even though the only work is gonna be to add a couple of default traits) seems redundant/not "artisan"-like code as every resource will have to have its controller and 99% of them will be literal copies with the included traits. We have this approach with non optional controllers and provided default traits in our own JSON API bundle (https://github.com/trikoder/jsonapibundle) that we made for our Symfony apps and I must say that I like like the current way how this package works a lot better. The approach with non optional controllers and provided default traits just lead to relatively big controllers where people would basically copy the entire trait method for the action in their controller just to edit a few lines for what they needed. So I'm speaking from experience when I say that I like the current way of this package a lot more. Of course it's kinda hard to comment or judge this approach without seeing at least some code of how 2.0 will look so maybe people won't get caught in the above trap if the system will be very easy to hook into at each step of the way. |
@X-Coder264 thanks, and thank you for taking the time to share your thoughts. You've made a really great point about the controllers. I think my thought process was around making content negotiation easier to wire in (it's very hacky at the moment to support other media types, though it does work). But actually sacrificing a good part of the package (not having to have controllers, as it's the exception rather than the rule that you'd need one) doesn't make sense as you say. So I think the design above can be amended to continue making them optional. However, I'll compose the default controller of traits, so people can use the action methods elsewhere if they want. I think the content negotiation will be easier to wire anyway because moving to specific form requests for resource types gives you a defined place to wire in customisations anyway. Also, by moving to an event-driven system rather than hooks, there should be less need to have a controller! I'll amend the above description with the changes. There will be |
Thanks a lot for maintaining and pushing this package forward! It makes developing JSON:APIs in Laravel a pleasure! The direction described in this issue seems to make sense. But to be honest it's a little bit different to get a full picture without seeing more implementation details.
Are we taking about OpenAPI Specification (Swagger)?
Why only for update events and not for create?
This would be awesome! But puts a lot of maintenance burden on this project, isn't it? Another approach would be codemods. I'm not that used to codemods in PHP but failed in love with them for JavaScript projects. There is a atanamo/php-codeshift package, which claims to provide a similar feature set and developer experience as |
Yes totally appreciate that... my intention with this issue is to float the direction and check people think it seems ok - before coding the implementation. So good to hear you think the direction makes sense.
My thinking is I can create a representation that can then be passed to whatever format you want to output it as. Having an OpenAPI spec writer would make sense. But the writer pattern would mean we're not tied to one output format.
Because only an update has before and after state. For a create, there's no state initially, so no concept of attributes that were changed. You would of course have access to what was created. To be clear, there will be
Yeah I use code mods all the time in Ember JS land. It's an idea that could be investigated. |
Just thinking about what would happen with sofdeletes. If we would handle them with a 'delete' event & 'change' event, we would end up with two events. Laravel only fires a 'deleted' event. Also would be nice to see Operations. I've already read your comment that you won't add them until they are final in the standard but there is a really big need for them. I know this is a big problem since they aren't even in v1.1. Is there a possibility where we could implement them? And thank your for this awesome package! We are in the final stages of our product development. If we are finished and we can make a living from it, we will definitely start contributing to this package! |
There is a high chance that Operations will be supported by an official extension in v1.1: json-api/json-api#1435 and json-api/json-api#1437 |
I'm using this package for new APIs at my company, having been writing very ad-hoc APIs using Laravel for a while now and wanting to move to something based on a spec. This package has been awesome and I really appreciate all the work that's gone into it! I have but one hopefully-humble request: We are currently structuring our code using the
Maybe those leaf files could still have a This blog post makes a great argument for structuring application code in this fashion as projects get larger. |
We also just started using this package for a project at my company, and I would be very happy to help in whatever way possible. |
@crabmusket I was about to mention the same thing and then saw your comment. I agree, if namespaces and file paths were more customizable, the more this package could be used with larger projects. @crabmusket I did set up a large project to use a module structure like your example. Also, going with your example I did use the Post prefix with the leaf files like you mention, same reasoning here too. I mostly took the shortest paths to hack this together and get it working but I could put together an example if it would help? It's mostly a combination of console commands to create the files in that structure and then a custom resolver to load them correctly. @lindyhopchris I just want to take this chance to thank you for this package. I've just integrated it into a big project which took a lot of digging into source to get all of the customization I need and it's been a pleasure to work with. Perfect for my project. I'm going to update to 2.0 right away and I'll leave any feedback or help if I can. 2.0 looks great. I also ran into issues with the |
Sorry for slow reply everyone! @crabmusket yes, I prefer the pod structure too, so I think I would like to support this (but maybe not as the default, as the other way is more Laravel-y). Potentially we should keep the @kdevan the resolution of files is already customisable, just not sure if I've written proper docs on it. The intention would be to leave the resolution behind an interface anyway, so this would continue to be customisable. Thanks for the great feedback btw, much appreciated! In terms of |
Thanks for the great work with the library. I've just started using it, but I love it so far 🙂 Regarding the documentation, you could use github pages and either https://docusaurus.io/ or http://docpress.github.io/index.html. I think both can be customized to look similar to Laravel documentation. Both require minimum setup time and will read |
@dingo-d great, thanks for those docs suggestions! |
Note to myself... the starter docs template for this also looks good: |
As an update for everyone on this at the moment... I'm incredibly pushed with work deadlines at the moment. Refactoring this package to improve it is definitely still needed (all the work projects I'm active on use it), but I need to be realistic about how much refactoring I can do over a period of time. Also I think it's probably best not to attempt to deliver all the above changes in one major version - because the upgrade path will be huge (and probably result with some people, including me, being stuck on Therefore the plan is still to deliver the above refactoring, but over a number of major versions... i.e. step the changes. I think the first place to start is to upgrade the |
How much support do you want from contributors in this process? I'm starting to make use of this package for new APIs where I work. I'd love to find some way to contribute, but getting my head around the code codebase isn't easy. Is there a specific issue you think would be useful to look into as a focus of my attention which would help me get up to speed? If nothing comes to mind, I think I'll just start to take a look at the neomerx package and get familiar with it, maybe implement some APIs using it only rather than the full automation of this package. |
@crabmusket yeah, i find it very difficult to involve contributors in the process because the codebase is really complex. It's got complex because originally it was framework-agnostic. Part of this future development plan is to strip it back so that it feels much more like a full Laravel package, which will hopefully make it a lot easier for Laravel developers to understand! My thinking is the first step is to do an upgrade of the If that's something you'd like to help with, then wrapping your head around the neomerx package is a good place to start. I'm hoping to have some time to work on this in the next couple of weeks. |
@lindyhopchris, its been a while, but still awesome how hard you’re continuing on this package. Although I would like to help regarding neomerx, a thing I found hard about it, is it’s code. I’ve been researching other packages related for this topic and see a pretty new one coming up but looks (can be deceiving) a easier to me. https://github.com/json-api-php/json-api Is that even a possibility to change your base all together to this? I want to dig into this myself first as well |
@Nizari thanks for suggesting that, haven't seen that before. The neomerx package is a complete headache to wrap your head round - not least because their coding style is different from the Laravel coding style. I've spent years using it so I understand what's going on inside of it, so I've kind of done the hard work around understanding it!! The upside is nemoerx is completely focused on making their encoder super performant. Particularly when iterating over large data sets to produce compound documents. This for me is the key thing: to use a different package, it would have to have that attention to performant encoding. I'll definitely take a look at the package you've linked to though. To be honest, my approach with the new version is going to be to hide whatever does the encoding behind an interface in this package, so that the use of neomerx is completely hidden. That would mean if we wanted at any point to use a different package for encoding, it could be swapped out behind that interface. |
I've started work on the next version of the encoder and other related clases. I know a number of people have offered to help in the past; the new code should be easier to wrap your head around. I've got a few things that are good for people to contribute, if people still want to volunteer. Here's an invite link to Slack @Nizari While I'm working on a |
Update on this. I've got a working encoder that hides the Neomerx encoder as an internal implementation detail. It's exceptionally quick, i.e. by upgrading to the latest version of the Neomerx package and by wiring into it in the way I'd envisaged, there doesn't seem to be any performance cost to developing our own approach that hides the Neomerx interfaces. A lot of extra stuff to do before a |
@lindyhopchris The Slack invite link is not active, can you re-share it again? |
@andresayej sorry! here's a new one: https://join.slack.com/t/laraveljsonapi/shared_invite/zt-e3oi2r4y-8nkmhzpKnPQViaXrkPJHtQ |
Hi @lindyhopchris We upgraded our application to Laravel 7 and a lot of tests were breaking, when we upgraded the package. After inspecting the code base we came aware of this issue and a lot of deprecations in the package. My question is, what is the plan on backwards compatibility and developer experience? We discovered the package and used it to build and test our API a couple months ago, but where to go from here? Rewrite the whole API again, or forking and maintaining our own version of this package? Maybe give the devs reading the docs a large hint, that when using the package, they are probably standing in front of a grand rewrite. Or am I misunderstanding the future development? |
@michaelklopf it's a great question. I'm not keen to introduce a painful upgrade path in this package, so my current tactic is as follows. I'm in the same situation as you that I have a number of production applications that are using this package and for which it would be a nightmare to introduce a painful upgrade path if there were a lot of changes in one major version step. As such I have therefore started to dev the new version in a separate package, under a Laravel JSON API Github organisation. That will allow the new version to be developed without disrupting this package. Once that new package has a settled approach, I can therefore start exploring a gradual upgrade path for this current package that is not massively disruptive and means developers have the option of gradually upgrading over time, rather than having to re-write to adopt the new package. That means this package will be maintained for the foreseeable future though it may not receive any new features. I.e. will be maintained with the current feature set. Does that make sense? Would be great to hear your thoughts! |
I like your approach. Probably the most sensible one. Similar to building APIs with your tool and not trying to break our users applications 😅 |
How about adding RQL (Resource query language) as JSON:API specification leaves the filtering logic to us =) I found some packages related to this which we can use to integrate. I know it has good amount of work with eloquent query builder. [Laravel] https://github.com/noitran/rql |
@smtlab thanks for the suggestion but I'm not keen on adding something specific for filtering, as it's really down to each application to decide how to implement filtering. I'd prefer to keep this package just specific to what's in the spec, not fill in gaps in it. |
@lindyhopchris I'm starting a new API and would like to use v2. Between the examples in this thread, docs, and then referencing code, what should I use to implement v2 right now? Just trying to separate out v1/v2 information. I'm digging into it now but figure some up-to-date context from you might help! Edit: looks like v2 is standard now :) I'll assume docs are all good then. |
@kdevan yeah you want to use the latest version of this package. I'm still dev'ing the completed refreshed package, but I'm doing it in a separate package. This package ( One the new package has settled and is production ready, I can then work on upgrade paths from this package to the new package. |
@lindyhopchris any update on the status of the refreshed package? |
Yeah I've been working on it - it's coming along nicely, I've got Nova style schemas working now which is good. It's just slow progress because I've only got limited time on Open Source work at the mo... but it is progressing. |
@lindyhopchris do you mean that we can just mention the field type and column name and the package will take care of the rest? If you are implementing it in a way how Laravel Nova works, It will be awesome. We can save a lot of time on while writing simple CURD API's and for more complex things we can use Actions as we have in Nova. Anyways I got excited when you mentioned nova. I'm available if you need some contribution in the code. |
@martianatwork yes it will be like that, but if you need to customise the resource object then you'd need to implement that. Here's an example of what it's looking like at the moment... I'm really tied up with work at the moment, so haven't had any open source time. I'm planning a sprint on Laravel JSON API in November, as I have a late-October work deadline. The schema is the Nova-like class: <?php
declare(strict_types=1);
namespace DummyApp\JsonApi\V1\Posts;
use DummyApp\Post;
use LaravelJsonApi\Eloquent\Contracts\Paginator;
use LaravelJsonApi\Eloquent\Fields\DateTime;
use LaravelJsonApi\Eloquent\Fields\Relations\HasMany;
use LaravelJsonApi\Eloquent\Fields\Relations\BelongsTo;
use LaravelJsonApi\Eloquent\Fields\Str;
use LaravelJsonApi\Eloquent\Filters\ID;
use LaravelJsonApi\Eloquent\Pagination\StandardPaginator;
use LaravelJsonApi\Eloquent\Schema;
class PostSchema extends Schema
{
/**
* The model the schema corresponds to.
*
* @var string
*/
protected string $model = Post::class;
/**
* The resource the schema corresponds to.
*
* @var string
*/
protected string $resource = PostResource::class;
/**
* @inheritDoc
*/
public function fields(): array
{
return [
BelongsTo::make('author')->inverseType('users')->readOnly(),
HasMany::make('comments')->readOnly(),
Str::make('content'),
DateTime::make('createdAt')->sortable()->readOnly(),
Str::make('synopsis'),
Str::make('title')->sortable(),
DateTime::make('updatedAt')->sortable()->readOnly(),
];
}
/**
* @inheritDoc
*/
public function filters(): array
{
return [
ID::make(),
];
}
/**
* @inheritDoc
*/
public function pagination(): ?Paginator
{
return StandardPaginator::make()->withMetaKey(null);
}
} It will be possible for the JSON to be generated just using the schema above. However, if you need to start customising the resource object, there will be a resource class that works like the Eloquent resources: <?php
declare(strict_types=1);
namespace DummyApp\JsonApi\V1\Posts;
use LaravelJsonApi\Core\Resources\JsonApiResource;
use function url;
class PostResource extends JsonApiResource
{
/**
* @return string
*/
public function type(): string
{
return 'posts';
}
/**
* @return string
*/
public function selfUrl(): string
{
return url('api/v1', [$this->type(), $this->id()]);
}
/**
* @return iterable
*/
public function attributes(): iterable
{
return [
'content' => $this->content,
'createdAt' => $this->created_at,
'synopsis' => $this->synopsis,
'title' => $this->title,
'updatedAt' => $this->updated_at,
];
}
/**
* @return iterable
*/
public function relationships(): iterable
{
return [
$this->relation('author'),
$this->relation('comments'),
];
}
} All the code above is written and working, just need a week of Open Source time to get to a releasable version. |
@lindyhopchris this sounds great, when can we test this? |
@martianatwork yes good point, should provide an update! I managed to spend several weeks on the revamped library in November. It's almost ready for a pre-release tag, which I'm hoping to do relatively soon. It's taken longer than I expected because I spent the time writing full documentation rather than launching it without docs. You can take a sneak peak here: I'll post here again when I release the first tagged version |
Have tagged the first alpha release of the redesigned package - available via Composer as Full documentation has been published here: Would be good if there were any keen early-adopters on here! As you'll see from the documentation, there's a lot in there already. Feedback required to work out what the priorities will be for what needs adding next. Join the conversation on Slack |
Plan for Version 2
We are starting to turn our attention to version 2 of this package. This issue describes the thinking and invites any comments/suggestions about the approach. We're aware a lot of people are using this package, so we want to get this right.
Background
When we first wrote this Laravel package, it was based on a framework-agnostic implementation because we weren't just using it in a Laravel application. One of the best decisions of the
1.0.0-alpha/beta
series was to move the code into this package and develop it as a solely-Laravel package.During the
1.0
development we started making it feel more like a Laravel package, however there was a limit to how much we could do that without introducing more breaking changes than we were comfortable with.The other bit of background is that we use the
neomerx/json-api
package, predominantly for the encoding. That package has a great attention on making encoding performant, so we still want to use it. However, we are not entirely confident that decisions made on that package fit with our overall approach and unfortunately recent changes to that package make extending some of the internals complex.Additionally, exposing the neomerx interfaces to the Laravel developer makes this package feel less intuitively like a Laravel package - as there's more code to wrap your head around.
Aims
Based on this background, there are two main things we want
2.0
to achieve:Laravel Alignment
Currently we have:
This will still work on
2.0
, requiring only minor interface changes and PHP 7 type-hinting to upgrade. Any code relating to the implementation will be marked as deprecated and removed in3.0
.Our new style for
2.0
will be this:The reason for the
JsonApi
namespaces is to prevent collisions with existing classes, e.gan Eloquent
PostResource
, or an existingPostRequest
form request. It also makes it entirely clear which classes are to do with your JSON-API implementation.This means the following differences from the current implementation:
Schema
is replaced byPostResource
, which will have a similar style to Eloquent resources. Similar is intentionally used, because the Eloquent approach will not fully work for JSON API, and it has some aspects that would undermine the performance of the JSON API encoder. Resource classes will implementResponsable
andJsonSerialize
so that they can be used in other contexts as required.Validators
is split intoPostRequest
for validating request JSON content andPostQuery
to validate the query parameters for a response that will containposts
resources.Authorizers
cease to exist and use standard Laravel authorization via middleware, form requestauthorize
methods, controller middleware and controller helpers.ContentNegotiators
are removed but the use-case is fully supported (and documented). Different media-types will be easier to wire in as there will be a specificPostRequest
andPostQuery
on which validation can be customised or skipped.null
. These will allow the adapter to providemeta
andlinks
as needed.Server
class, so it's easier to call our default handling of JSON API requests from within a custom controller if needed.As an example, our new default controller would look like this:
This means that if the developer wanted to write their own controller, changing the method signatures of some of the actions, they could compose their own controller using our traits - but omit the ones they want to change. E.g.
Outside of HTTP requests, you can query your resources using a fluent JSON API query syntax. This effectively allows you to run the logic within your adapters in a style familiar to a Laravel Query Builder (but using JSON API terms, such as
include
). Say we wanted to get aPostResource
from a post id:Neomerx Package
We are currently using v1 of the neomerx package, but it is currently on v3 - which contains a lot of performance improvements. We therefore desperately need to upgrade.
However, as the current implementation involves developers extending or type-hinting classes/interfaces from the neomerx package, this change will be extremely breaking.
As we plan to change our use of the neomerx package to an internal implementation detail,
2.0
of our package will set the neomerx Composer constraint to^1.0.3|^3.0
. We will detect the version installed, caching the answer in config for performance reasons, and adjust the implementation internally to use either.Developers will therefore be able to upgrade to
2.0
without changing their existingSchemas
etc. And can then upgrade to neomerx v3 once they have converted to the newPostResource
pattern described above.How Can You Help?
Provide comments on this approach and highlight potential problems, so that we know whether we're on the right track!
We think that with these changes, this package will be one of the best options out there for building standard-compliant APIs in Laravel. So we could really do with a great website with docs that follow the Laravel docs sub-headings (i.e. so that they are instantly familiar to Laravel developers). If you're able to help out with a public website, let me know.
The text was updated successfully, but these errors were encountered: