Skip to content

Conversation

@JRogaishio
Copy link
Contributor

@JRogaishio JRogaishio commented Oct 4, 2024

Checklist

  • Tests
  • Docs
  • Type definitions

Changes:

  • Added wrap, _unwrap, wrappedBy, and nowrap methods to Model.js to mimic Laravel's resource data wrapping. This makes for $first, $find, $get, and $all obsolete but I kept them for backward compatibility. The the overloadable wrap() allows you to automatically unwrap using the standard first, find, get, all methods but also change what the wrapper key is in case you don't want to use the standard 'data' property
  • Lock the defu package to 6.1.2. Version 6.1.3+ has a regression that prevents exported objects from merging with plain objects.
  • Added unit tests for the new wrap() method
  • Added new wrap() type
  • Added new wrappedBy(wrap) and nowrap() builder methods to change / disable wrapping
  • Updated documentation for new wrap() type
  • Updated documentation for new wrappedBy() and nowrap() methods

@JRogaishio JRogaishio changed the title Feat/resource wrapper feat(model): add method wrapper to define the api response wrapper / overrides Oct 4, 2024
…avel's resource data wrapping. This is a shortcut for , , , that allows you to not only automatically unwrap using the standard first, find, get, all methods but also change what the wrapper key is in case you don't want to use the standard 'data' property
…ssion that prevents exported objects from merging with plain objects.
@JRogaishio JRogaishio force-pushed the feat/resource-wrapper branch from 1ad80d5 to 602a787 Compare October 4, 2024 17:24
…ssion that prevents exported objects from merging with plain objects. Also include package-lock.json for npm users

Choose a reason for hiding this comment

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

This file should be excluded from the commit

Choose a reason for hiding this comment

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

This file should be excluded from the commit

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Resolved. yarn.lock has been removed and also added to the .gitignore so it doesn't happen again.

@JRogaishio
Copy link
Contributor Author

JRogaishio commented Nov 1, 2024

I updated this PR to also include .nowrap() and wrappedBy(v) methods since sometimes (Like for paginated responses) you want to disable wrapping at runtime for a single response.

@robsontenorio
Copy link
Owner

cc @JoaoPedroAS51

@HashGateApp
Copy link

@robsontenorio Who should be the reviewer of this PR?

@JoaoPedroAS51
Copy link
Collaborator

Hi guys, sorry for the delay. I've been very busy lately. I'll review the PR.

@JRogaishio thank you for the PR! :)

@JoaoPedroAS51 JoaoPedroAS51 self-requested a review November 7, 2024 20:51
@robsontenorio
Copy link
Owner

@JRogaishio On sidebar you need to allow edit/commits by maintainers.

So @JoaoPedroAS51 can go ahead with this.

image

@JRogaishio
Copy link
Contributor Author

@robsontenorio I don't think I can give maintainer access to my fork / pr since it's on an organization and not user owned. The option just doesn't exist on the PR (see below screenshot). I did however resolve the merge conflicts and brought yarn.lock back since it appears you're using it.
image

Docs I'm trying to follow:
https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/allowing-changes-to-a-pull-request-branch-created-from-a-fork

On user-owned forks, if you want to allow anyone with push access to the upstream repository to make changes to your pull request, select Allow edits from maintainers.

@JoaoPedroAS51
Copy link
Collaborator

Hi @JRogaishio! Thank you for fixing the conflict! Yeah, we only save the yarn.lock in the repo, which is the package manager we use for this project. Can you just revert the revert the .gitignore? Then I think we are good to go. Many thanks! :)

@JRogaishio
Copy link
Contributor Author

@JoaoPedroAS51 You're welcome. Also the .gitignore has been reverted back to the robsontenorio/dev version so it should be all set now.

@JoaoPedroAS51 JoaoPedroAS51 merged commit 6917338 into robsontenorio:dev Nov 8, 2024
6 checks passed
@JoaoPedroAS51
Copy link
Collaborator

Hi guys, sorry for not releasing it yet. It seems that the release action failed, I'll try to take a look over the weekend.

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.

4 participants