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

Typescript: Allow arbitrary model updates, not only Collections #1066

Merged
merged 4 commits into from
Nov 14, 2022

Conversation

lanhhv84
Copy link
Contributor

@lanhhv84 lanhhv84 commented Nov 12, 2022

  • Given:
type Model =  {
    items: HasMany<Item>
}
  • Current behavior:
ModelInstance<Model>.update({items: [item1, item2]}); // $ExpectError
ModelInstance<Model>.update({items: Collection<Item>});
  • New behavior:
ModelInstance<Model>.update({items: [item1, item2]}); 
ModelInstance<Model>.update({items: Collection<Item>}); 
  • Motivation: We should be able to use readily available/simple data types to update our models, instead of having to create a new Collection first before we can update.

@IanVS
Copy link
Collaborator

IanVS commented Nov 13, 2022

I guess this would be a breaking change for anyone providing items as a collection?

@lanhhv84
Copy link
Contributor Author

Good point! I will update the code to suport both Array and Collection. Thanks.

@lanhhv84
Copy link
Contributor Author

Thanks for your feedback. I updated the code to remove breaking changes.

@lanhhv84 lanhhv84 marked this pull request as draft November 13, 2022 08:22
@lanhhv84 lanhhv84 marked this pull request as ready for review November 13, 2022 08:22
Copy link
Collaborator

@IanVS IanVS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me, but it seems like maybe the documentation is incorrect. It seems to indicate that the second argument to update should only be a string. Or am I understanding it incorrectly? model.update I'm guessing the docs are just wrong, ya? If so, do you think you could find and fix that as well? It should be here in this repo.

@lanhhv84
Copy link
Contributor Author

Great catch! The documentation for that function should be updated with the code now.

@IanVS IanVS merged commit 8d34708 into miragejs:master Nov 14, 2022
@IanVS IanVS changed the title Improve ModelInstance<Model>.update Typescript: Allow arbitrary model updates, not only Collections Nov 14, 2022
@IanVS
Copy link
Collaborator

IanVS commented Nov 14, 2022

Thanks for the improvements!

@lanhhv84 lanhhv84 deleted the lanh/improve-typing branch November 14, 2022 13:20
kevinansfield pushed a commit to TryGhost/Ghost that referenced this pull request Feb 28, 2023
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [miragejs](https://togithub.com/miragejs/miragejs) | [`0.1.45` ->
`0.1.47`](https://renovatebot.com/diffs/npm/miragejs/0.1.45/0.1.47) |
[![age](https://badges.renovateapi.com/packages/npm/miragejs/0.1.47/age-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://badges.renovateapi.com/packages/npm/miragejs/0.1.47/adoption-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://badges.renovateapi.com/packages/npm/miragejs/0.1.47/compatibility-slim/0.1.45)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://badges.renovateapi.com/packages/npm/miragejs/0.1.47/confidence-slim/0.1.45)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>miragejs/miragejs</summary>

###
[`v0.1.47`](https://togithub.com/miragejs/miragejs/releases/tag/v0.1.47)

[Compare
Source](https://togithub.com/miragejs/miragejs/compare/v0.1.46...v0.1.47)

🚀 Enhancements

- Typescript: Allow arbitrary model updates, not only Collections by
[@&#8203;lanhhv84](https://togithub.com/lanhhv84) in
[miragejs/miragejs#1066

🐛 Bugfixes

- Establish precedence for embed over serializeIds by
[@&#8203;NoneOfMaster](https://togithub.com/NoneOfMaster) in
[miragejs/miragejs#1074

#### New Contributors

- [@&#8203;lanhhv84](https://togithub.com/lanhhv84) made their first
contribution in
[miragejs/miragejs#1066

**Full Changelog**:
miragejs/miragejs@v0.1.46...v0.1.47

###
[`v0.1.46`](https://togithub.com/miragejs/miragejs/releases/tag/v0.1.46)

[Compare
Source](https://togithub.com/miragejs/miragejs/compare/v0.1.45...v0.1.46)

#### What's Changed

🚀 **Enhancements**

- Support selected embedded relationships by
[@&#8203;NoneOfMaster](https://togithub.com/NoneOfMaster) in
[miragejs/miragejs#850
- Add resource function type definition by
[@&#8203;morganmspencer](https://togithub.com/morganmspencer) in
[miragejs/miragejs#1059

🐛 **Bugfixes**

- Fixed timing being incorrectly passed to pretender by
[@&#8203;cah-brian-gantzler](https://togithub.com/cah-brian-gantzler) in
[miragejs/miragejs#1051
- Fix includes query params by
[@&#8203;mansona](https://togithub.com/mansona) in
[miragejs/miragejs#1064

🗂 **Types**

- Align passthrough type declaration with implementation by
[@&#8203;brzosthub](https://togithub.com/brzosthub) in
[miragejs/miragejs#1061

#### New Contributors

- [@&#8203;brzosthub](https://togithub.com/brzosthub) made their first
contribution in
[miragejs/miragejs#1061
- [@&#8203;NoneOfMaster](https://togithub.com/NoneOfMaster) made their
first contribution in
[miragejs/miragejs#850
- [@&#8203;morganmspencer](https://togithub.com/morganmspencer) made
their first contribution in
[miragejs/miragejs#1059
- [@&#8203;mansona](https://togithub.com/mansona) made their first
contribution in
[miragejs/miragejs#1064

**Full Changelog**:
miragejs/miragejs@v0.1.45...v0.1.46

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "every weekday" (UTC), Automerge - At
any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://app.renovatebot.com/dashboard#github/TryGhost/Ghost).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC4xOS4wIiwidXBkYXRlZEluVmVyIjoiMzQuMTE2LjEifQ==-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
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

Successfully merging this pull request may close these issues.

None yet

2 participants