contentful / experience-builder

https://www.contentful.com/developers/docs/experiences/what-are-experiences/
MIT License
6 stars 1 forks source link

feat: surface state to use experience builder [SPA-1454] #109

Closed Spring3 closed 9 months ago

Spring3 commented 9 months ago

Please check the README file to see the new examples of the configuration

This is the first bite at the problems that surfaced when trying to use the sdk with next.js in ssr mode.

Specifically the main changes:

Next steps (in future PRs):

Spring3 commented 9 months ago

changed to feat from refactor to ensure the version bump by semantic release after merge

dimitrycf commented 9 months ago

Pulling up of the client makes initalization of the SDK look much cleaner 🤩

Having client pulled up makes it for better separation of concerns:

Also dropping import of useComponents() hook is great as less lines to import and also we focus on just two core imports import { CompositionRoot, useExperienceBuilder } from 'experience-builder' 👍

// ...
const client = createClient({
  space: process.env.CTFL_SPACE_ID,
  environment: process.env.CTFL_ENV_ID,
  host: process.env.CTFL_API_HOST
  accessToken: process.env.CTFL_TOKEN, token if 'cdn.contentful.com'
});

const App = () => {

  const { settings, experience, defineComponent } = useExperienceBuilder({
    client, 
    experienceTypeId: process.env.CTFL_EXPERIENCE_TYPE_ID,
    defaultLocale: 'en-US',
    slug: 'landing-page', 
    mode: 'delivery'
  })

  useEffect(() => {
    defineComponent('Button', componentDefinition)
  }, [defineComponent])

  // ...

  return <CompositionRoot settings={settings} experience={experience} />
}

this is much cleaner than previous SDK initialization snippet, where all the client initialization params were mixed with other params when initializing SDK via useExperienceBuilder()

Old snippet for comparison

const spaceId = process.env.REACT_APP_SPACE_ID || ''
  const environmentId = process.env.REACT_APP_ENVIRONMENT_ID || 'master'
  const accessToken = process.env.REACT_APP_PREVIEW_ACCESS_TOKEN
  const host = process.env.REACT_APP_PREVIEW_HOST

  const { 
        experience, 
        locale,
    } = useExperienceBuilder({
    host,
    accessToken,
    spaceId,
    environmentId,

    experienceTypeId,
    initialLocale,

    initialMode: 'preview',
  });

    const { defineComponent } = useExperienceBuilderComponents();

    useEffect(()=>{
        defineComponent(MyCounter, myCounterComponentDefinition);
    }, [defineComponent]);

  return (
    <CompositionRoot experience={experience} locale={locale} slug={slug} />
  );
dimitrycf commented 9 months ago

Video narration of the most important parts of the PR

For the sake of maintaining my own personal sanity when reviewing large PRs I have a habit of narrating (and often recording) how I navigate around the code (to cause the "rubber duck debugging" effect and also to have a point of reference that later I can rewatch and recall what the PR was about).

Here I am sharing this video where I navigate around the code and narrate the most important facts. Maybe it helps someone to find their way around the PR.

https://www.loom.com/share/f1f52b24294d49fcad9ba1e461fdc3db

Spring3 commented 9 months ago

@dimitrycf

This example was obviously an example that such possibility exists.

Screenshot 2023-09-04 at 14 59 10

What I meant is for example this:

return (
   <Nav>
      <LocaleSwitch onChange={(locale) => settings.setLocale(locale)} />
   </Nav>
   <CompositionRoot experience={experience} settings={settings} />
)

It just didn't want to write imaginable code, because there can be so many ways and cases, so I narrowed it down just to that example as you see on the photo to explain that such change can be possible

SofiaMargariti commented 9 months ago

@Spring3 I think it's a good idea that you moved the slug back to tag level. I was testing this on the demo website and it was a bit awkward because I had to call useExperienceBuilder early in the code in order to use defineComponent but at that point I didn't have the slug available yet. Alternatively I could call useExperienceBuilder inside each individual Route but then I'd have to duplicate the component definition code.

Spring3 commented 9 months ago

@Spring3 I think it's a good idea that you moved the slug back to tag level. I was testing this on the demo website and it was a bit awkward because I had to call useExperienceBuilder early in the code in order to use defineComponent but at that point I didn't have the slug available yet. Alternatively I could call useExperienceBuilder inside each individual Route but then I'd have to duplicate the component definition code.

all credits go to @dimitrycf who initially brought this point in slack DMs

Glad to hear that it was a change for better :)

SofiaMargariti commented 9 months ago

I noticed that when when there is not composition for a specific slug, the sdk tries to refetch endlessly. Have you seen this? See the console errors here:

https://github.com/contentful/experience-builder/assets/2773012/305f2a30-be0a-49ad-a198-138f3dfdc786

Spring3 commented 9 months ago

I noticed that when when there is not composition for a specific slug, the sdk tries to refetch endlessly. Have you seen this? See the console errors here:

Screen.Recording.2023-09-04.at.17.16.06.mov

oof, no. But I can see why. I'll check it tomorrow, thanks for finding it

dimitrycf commented 9 months ago

I noticed that when when there is not composition for a specific slug, the sdk tries to refetch endlessly. Have you seen this? See the console errors here: Screen.Recording.2023-09-04.at.17.16.06.mov

oof, no. But I can see why. I'll check it tomorrow, thanks for finding it

I have seen this as well last week. I think it's because of dependency on experience. And it has experience.error which is set upon failure to load compositon; which causes infinite useEffect() loop image

dimitrycf commented 9 months ago

As discussed with @Spring3 I have put my suggestions into this PR for Daniel to review

contentful-automation[bot] commented 9 months ago

:tada: This PR is included in version 1.1.0-alpha.63 :tada:

The release is available on:

Your semantic-release bot :package::rocket: