prismicio / prismic-react

React components and hooks to fetch and present Prismic content
https://prismic.io/docs/technologies/homepage-reactjs
Apache License 2.0
154 stars 40 forks source link

SliceZone component is rendering slices in the wrong order in Preview #101

Closed kb1995 closed 2 years ago

kb1995 commented 2 years ago

Versions

When not in Preview, the content is rendered correctly in the correct order. However, when in preview, there are couple of Slices which shouldn't be on the top, appearing on the top.

My SliceZone Resolve

import { SliceZone } from '@prismicio/react';
import SliceCarousel from './slices/SliceCarousel/SliceCarousel';
import SliceKtpHeroSection from './slices/SliceKtpHeroSection/SliceKtpHeroSection';

// code split larger components that are below the fold
const SliceCTASection = loadable(() => import('./slices/SliceCTASection/SliceCTASection'));
const SliceAuthorSection = loadable(() => import('./slices/SliceAuthorSection/SliceAuthorSection'));
const SliceTableOfContents = loadable(() =>
  import('./slices/SliceTableOfContents/SliceTableOfContents')
);
const SliceChapterInfo = loadable(() => import('./slices/SliceChapterInfo/SliceChapterInfo'));
const SliceRichText = loadable(() => import('./slices/SliceRichText/SliceRichText'));
const SliceQuote = loadable(() => import('./slices/SliceQuote/SliceQuote'));
const SliceImage = loadable(() => import('./slices/SliceImage/SliceImage'));
const SliceTextWithIcon = loadable(() => import('./slices/SliceTextWithIcon/SliceTextWithIcon'));
const SliceTableTwoCols = loadable(() => import('./slices/SliceTableTwoCols/SliceTableTwoCols'));
const SliceSectionTitle = loadable(() => import('./slices/SliceSectionTitle/SliceSectionTitle'));
const SliceStoryCards = loadable(() => import('./slices/SliceStoryCards/SliceStoryCards'));
const SliceSpacing = loadable(() => import('./slices/SliceSpacing/SliceSpacing'));
const SliceStoryCardLink = loadable(() => import('./slices/SliceStoryCardLink/SliceStoryCardLink'));
const SliceCodeHighlight = loadable(() => import('./slices/SliceCodeHighlight/SliceCodeHighlight'));
const SliceGridCardList = loadable(() => import('./slices/SliceGridCardList/SliceGridCardList'));
const SliceImageContent = loadable(() => import('./slices/SliceImageContent/SliceImageContent'));

const SliceZoneResolver = ({ slices }) => {
  return (
    <>
      {console.log(slices)}
      <SliceZone
        slices={slices}
        components={{
          cta_section: SliceCTASection,
          ktp_hero_section: SliceKtpHeroSection,
          author_section: SliceAuthorSection,
          table_of_contents: SliceTableOfContents,
          chapter_info: SliceChapterInfo,
          rich_text: SliceRichText,
          quote: SliceQuote,
          image: SliceImage,
          text_with_icon: SliceTextWithIcon,
          table_2col: SliceTableTwoCols,
          section_title: SliceSectionTitle,
          story_cards: SliceStoryCards,
          spacing: SliceSpacing,
          carousel: SliceCarousel,
          story_card_link: SliceStoryCardLink,
          code_highlight: SliceCodeHighlight,
          grid_card_list: SliceGridCardList,
          image_content: SliceImageContent,
        }}
      />
    </>
  );
};

console.log(slices) renders the content in the correct order, however they don't appear in the same order in the browser

I'm getting several warnings in the console.

react-dom.development.js:67 Warning: Encountered two children with the same key, `{"slice_type":"spacing","slice_label":null,"__typename":"PrismicHowToBuildPageDataBodySpacing","id":"261d00221efe0426cb32c3df30de147a","primary":{"spacing":"medium"},"items":[{}]}`. Keys should be unique so that components maintain their identity across updates. Non-unique keys may cause children to be duplicated and/or omitted — the behavior is unsupported and could change in a future version.
angeloashmore commented 2 years ago

That's odd. It sounds like the component keys are causing ordering issues when the preview data replaces the static data.

I'll investigate and get back to you.

Since you are using Gatsby (correct me if I'm wrong), you have access to an id field for each Slice. This is Gatsby-specific, but with that, we could add a prop that lets you use that ID as the Slice's component key. We would still need to find a way to support non-Gatsby usage, however.

kb1995 commented 2 years ago

@angeloashmore Yes, I think that will be a nice solution for this. We need to make sure to mention in the docs that it's advisable to query the ID as part of the graphql query. Myself I usually just query slice_label and slice_type and often omit the ID.

I just had a look at the source code. The way that you generate the slice is using const key = JSON.stringify(slice);

This means that if I have 2 slices with the exact same data and I don't query the ID, then they would have the same key. For example, a simple Spacing slice. I assume the problem would be solved if we include ID in all of our Slice queries.

A potential solution would be to create the key logic using slice-${slice.slice_type}-${index} -> this is the logic I'm currently using for my custom SliceZone and it's been working quite well for me.

angeloashmore commented 2 years ago

@kb1995 Could you try v2.0.0-beta.7 and let me know if that fixes your issue?

I was able to reproduce the same issue. To fix it, I prepended the Slice's index to the key and kept JSON.stringify, similar to what you suggested. Using the full object in the key makes sure Slices with the same type and index won't have the same key.

(Imagine, for example, you have two Slices with the same type but different content in the fields. If we didn't account for the content fields, the components may not be correctly identified since the keys could overlap.)

I think using this strategy should work for all cases. I did not expose a getKey prop that lets you give a function since that could cause confusion for some users. If we come across this issue again, however, then adding an explicit prop to define your own key may be required.

kb1995 commented 2 years ago

@angeloashmore I finally came around upgrading the package and from the looks of it, the bug is fixed. Thanks!

I agree - even my case is already an edge case, so I doubt this will be a common case.

angeloashmore commented 2 years ago

Awesome, thanks for testing it out! I'll keep this issue in mind as others start to try out the updated library.