Skip to content

Conversation

@gautamsi
Copy link
Member

@gautamsi gautamsi commented Jul 8, 2024

This replaces all the hooks from original combined form with the granular one inline with #9056 #9057 #8826

this cleans up the code and is breaking changes.

@codesandbox-ci
Copy link

codesandbox-ci bot commented Jul 8, 2024

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 2676f4e:

Sandbox Source
@keystone-6/sandbox Configuration

@dcousens
Copy link
Member

dcousens commented Jul 8, 2024

I'll be merging #9166 ASAP, which should help with this pull request, fwiw

@gautamsi
Copy link
Member Author

gautamsi commented Jul 8, 2024

If we go the way of this PR, #9166 would not be needed I think.

This should be part of v-next branch so that it can take longer, you should merge #9166

@gautamsi gautamsi changed the base branch from main to v-next July 8, 2024 05:10
@gautamsi gautamsi force-pushed the operation-fields-hooks branch 2 times, most recently from 73b2403 to e2f6ad3 Compare July 18, 2024 01:55
@gautamsi gautamsi force-pushed the operation-fields-hooks branch from e2f6ad3 to f5e3425 Compare July 18, 2024 02:06
@gautamsi gautamsi marked this pull request as ready for review July 18, 2024 02:11
@gautamsi gautamsi changed the title [WIP] Remove deprecated hooks Remove deprecated hooks Jul 18, 2024
@gautamsi gautamsi changed the base branch from v-next to main July 18, 2024 14:53
@gautamsi gautamsi changed the base branch from main to v-next July 18, 2024 14:53
@gautamsi
Copy link
Member Author

@dcousens I am done with this, are you able to take a look and help resolve single type error in lint and documentation.

@gautamsi
Copy link
Member Author

I have created documentation in separate PR #9231

@gautamsi gautamsi mentioned this pull request Jul 29, 2024
15 tasks
@gautamsi gautamsi force-pushed the operation-fields-hooks branch from c4d18a2 to 3861fd2 Compare August 10, 2024 17:51
@gautamsi gautamsi force-pushed the operation-fields-hooks branch from 3861fd2 to 38b2741 Compare August 27, 2024 11:33
@gautamsi gautamsi force-pushed the operation-fields-hooks branch from 38b2741 to 7a54a00 Compare September 5, 2024 02:42
@gautamsi gautamsi changed the base branch from v-next to main December 2, 2024 02:28
@gautamsi gautamsi force-pushed the operation-fields-hooks branch 2 times, most recently from 9cbda81 to 2676f4e Compare December 2, 2024 02:32
@gautamsi
Copy link
Member Author

gautamsi commented Dec 2, 2024

@dcousens changing base on this to cleanup from deprecated hooks. This has nothing to do with the nextjs upgrade and can be merged sooner for next major release.

@gautamsi gautamsi force-pushed the operation-fields-hooks branch 2 times, most recently from ee3fcc7 to 248fae3 Compare December 14, 2024 03:09
@gautamsi
Copy link
Member Author

let me know if you have any plan with this or waiting for something.

I am unable to fix the typing error if you are waiting for that.

@dcousens
Copy link
Member

I plan to merge this very soon

@dcousens
Copy link
Member

dcousens commented Jan 30, 2025

I'll happily rebase this soon, should be straight forward - prioritizing the DS upgrade landing for now

@dcousens dcousens force-pushed the operation-fields-hooks branch from 7ff0241 to fcb03ee Compare February 4, 2025 09:25
@gautamsi gautamsi force-pushed the operation-fields-hooks branch from fcb03ee to a80a8b5 Compare February 6, 2025 03:18
@gautamsi
Copy link
Member Author

gautamsi commented Feb 6, 2025

@dcousens I see that you have already rebased this.

You will have to help with the type issue in linting, it is something I am unable to find solution to.

@gautamsi
Copy link
Member Author

@emmatown I fixed structure field. Please check linting for select field.

@emmatown emmatown force-pushed the operation-fields-hooks branch from 151347a to ed57d18 Compare February 12, 2025 02:24
@emmatown emmatown requested a review from dcousens February 12, 2025 02:35
@gautamsi
Copy link
Member Author

@emmatown there is no point of the PR if you want to keep the top level hooks.

@gautamsi
Copy link
Member Author

if you are going to keep top level hooks, revert/update #9231

@dcousens
Copy link
Member

dcousens commented Feb 12, 2025

@gautamsi this removes the deprecated validateInput and validateDelete functions, and then introduces the breaking change to support the field hooks to use the same object notation as list hooks.

Unpacking the object is often the preferred approach beyond any level of complexity, so I think #9231 stays completely relevant?
Why remove the Function | { ... } hook syntax?

@emmatown emmatown force-pushed the operation-fields-hooks branch from 40bfac1 to c3c565d Compare February 12, 2025 05:57
@dcousens
Copy link
Member

dcousens commented Feb 12, 2025

Why remove the Function | { ... } hook syntax?

[Replying to myself] we did actually talk about this at length, but opted to keep things aligned with the approach in .access and everywhere else in the project for now.

Maybe in the future a better pattern will emerge where we only retain the unpacked type variants in the unresolved configuration, and some convenience function (like allOperations<F> (f: F) for access control) could be introduced.

@emmatown emmatown merged commit 77bfd2c into keystonejs:main Feb 12, 2025
45 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants