-
-
Notifications
You must be signed in to change notification settings - Fork 103
[Store] Introduce EmbeddableDocumentInterface
#707
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
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Embeddable
interfaceEmbeddable
interface
Not sure why PHPStan fails on 8.4 -- is it a linking issue? The argument it's complaining about seems fine to me. How do we feel about the rest? < 8.4 is going to fail because of the interface properties. Should we support 8.4 or should I solve this with getter methods on the interface instead of interface properties to accommodate for the earlier versions? |
chr-hertel
reviewed
Sep 30, 2025
b293f3a
to
81b8bac
Compare
OskarStark
reviewed
Oct 1, 2025
OskarStark
reviewed
Oct 1, 2025
GromNaN
reviewed
Oct 1, 2025
Embeddable
interfaceEmbeddable
interface
40c11b9
to
cd35c6e
Compare
OskarStark
reviewed
Oct 3, 2025
a13eb9f
to
10608ce
Compare
OskarStark
reviewed
Oct 6, 2025
Embeddable
interfaceEmbeddableDocumentInterface
OskarStark
approved these changes
Oct 6, 2025
EmbeddableDocumentInterface
EmbeddableDocumentInterface
10608ce
to
3e15fd5
Compare
Thank you Pauline. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In an effort to support user land objects for the store, this PR introduces an
Embeddable
interface instead of relying on theTextDocument
class. Please note this interface would require php ^8.4, as it uses interface properties. I chose this approach so as to introduce minimal change to the existingTextDocument
class and usages. Instead ofcontent
, anEmbeddable
document has adata
property. Any type embeddable document class can implement a hook for thedata
property if needed.This PR also introduces union types for the document IDs so as to not make assumptions about the ID type.