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

[draft] Split CmsQueryRepeater into CmsQuery and CmsQueryRepeater #62

Closed
wants to merge 1 commit into from

Conversation

zvictor
Copy link
Contributor

@zvictor zvictor commented Dec 9, 2022

The goal is to be able to run queries against the CMS reusing the currently developed code.

AFAIK there is only CmsQueryRepeater providing a convenient way to run such queries in React, but it has the limitation that it forces the design of the application to follow a separation of rows. Sometimes we want to run some aggregation on that data, or something else that does not expect to present one element per row.

This PR attempts to make the following code possible:

import { CmsQuery } from '@plasmicpkgs/plasmic-cms'

interface InboxProps {
  userId: string
}

const Notifications = ({ userId }: InboxProps) => (
  <CmsQuery table={'messages' as const} filterField={'user' as const} filterValue={userId}>
    {(rows) => <div>{rows.length} messages in your inbox</div>}
  </CmsQuery>
)

export default Notifications

@vercel
Copy link

vercel bot commented Dec 9, 2022

@zvictor is attempting to deploy a commit to the Plasmic Team on Vercel.

A member of the Team first needs to authorize it.

@vercel
Copy link

vercel bot commented Dec 9, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
contenful-example ✅ Ready (Inspect) Visit Preview Dec 9, 2022 at 4:24PM (UTC)
dynamic-multistep-forms ✅ Ready (Inspect) Visit Preview Dec 9, 2022 at 4:24PM (UTC)
plasmic ✅ Ready (Inspect) Visit Preview Dec 9, 2022 at 4:24PM (UTC)
plasmic-blog-example ❌ Failed (Inspect) Dec 9, 2022 at 4:24PM (UTC)
plasmic-blog-starter ❌ Failed (Inspect) Dec 9, 2022 at 4:24PM (UTC)
vercel-workflow ✅ Ready (Inspect) Visit Preview Dec 9, 2022 at 4:24PM (UTC)

@zvictor
Copy link
Contributor Author

zvictor commented Dec 9, 2022

My understanding of the code is that it should not break anything. However, when I test it, all calls to CmsQueryRepeater stop working.

I am not sure if it's a problem with my code or with the way I am testing it.
None of the CMS data appears in the $ctx with my code.

image

@yang
Copy link
Contributor

yang commented Dec 19, 2022

Thank you @zvictor!

We are trying to avoid making dramatic restructuring of the CMS components since all current projects depend on the current components and their interfaces (at least not without migrations).

However, your change prompted us to discuss and introduce an alternative approach to accomplish what you are mentioning as follows, using a noAutoRepeat prop—this would make the CMS components more consistent with our other data source integration components:

<CmsQueryRepeater ... noAutoRepeat>
  {/* Now you can do whatever you want from here */}
  <DataCtxReader>{ctx => ...ctx.plasmicCmsProductsCollection}</DataCtxReader>
</CmsQueryRepeater>

(You may also be interested in the noLayout prop if you want it to be a pure data provider with no HTML elements.)

cc @tmadeira for visibility!

@IcaroG
Copy link
Contributor

IcaroG commented Dec 19, 2023

Implemented in No auto-repeat and No layout.

@IcaroG IcaroG closed this Dec 19, 2023
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.

3 participants