-
Notifications
You must be signed in to change notification settings - Fork 61
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
fix: type PrismicDocument.*_publication_date
as TimestampField<"filled">
#304
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
@lihbr Could you take one more look? The most recent commit (see here) shows how I removed Doing so required using CI fails because (I assume) |
Codecov Report
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. @@ Coverage Diff @@
## master #304 +/- ##
=======================================
Coverage 99.94% 99.94%
=======================================
Files 49 49
Lines 5764 5764
Branches 309 308 -1
=======================================
Hits 5761 5761
Misses 3 3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All good dropping Node 14!
For the @ts-expect-error
, maybe let's just not add them, ignore the CI & publish manually (bypassing part of the release script)?
Types of changes
Description
This PR changes the following fields from
string
toTimestampField<"filled">
:PrismicDocument.first_publication_date
PrismicDocument.last_publication_date
Retyping the properties makes them compatible with
asDate()
when type-checked.Requested by @chamois-d-or
Discussion points
Will this have negative downstream effects?
Changing the types makes the properties more specific, which shouldn't affect user code that accepted one of the properties. It could, however, cause issues in edge cases where user code was overriding the properties.
Is there a better way to install
@prismicio/client
(i.e. itself) as@prismicio/mock
's peer dependency?@prismicio/mock
was updated with a peer dependency on@prismicio/client
. npm was installing the published version alongsidemock
, which did not include the updated types.Because this PR changes
PrismicDocument
types, it caused type incompatibilities between@prismicio/mock
andsrc/
version of@prismicio/client
.I was able to solve the issue by pointing the
@prismicio/client
dependency on itself via"@prismicio/client": "."
, which is a very odd setup.Checklist:
🦒