Skip to content

feat(client): add channelId as an optional param#994

Merged
jorgemoya merged 3 commits into
mainfrom
add-channelId-param-client
Jun 27, 2024
Merged

feat(client): add channelId as an optional param#994
jorgemoya merged 3 commits into
mainfrom
add-channelId-param-client

Conversation

@jorgemoya

@jorgemoya jorgemoya commented Jun 10, 2024

Copy link
Copy Markdown
Contributor

What/Why?

  • Add optional channelId param to client to allow fetching from multiple channels with the same client.
  • Remove non channel-aware endpoint.
  • Enforce channelId being used.

Testing

TBD

@jorgemoya
jorgemoya requested a review from a team June 10, 2024 19:28
@vercel

vercel Bot commented Jun 10, 2024

Copy link
Copy Markdown

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

Name Status Preview Comments Updated (UTC)
catalyst-latest ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 26, 2024 8:25pm
5 Ignored Deployments
Name Status Preview Comments Updated (UTC)
catalyst-1millionproducts-store ⬜️ Ignored (Inspect) Visit Preview Jun 26, 2024 8:25pm
catalyst-au ⬜️ Ignored (Inspect) Visit Preview Jun 26, 2024 8:25pm
catalyst-test-store ⬜️ Ignored (Inspect) Visit Preview Jun 26, 2024 8:25pm
catalyst-uk ⬜️ Ignored (Inspect) Visit Preview Jun 26, 2024 8:25pm
catalyst-unstable ⬜️ Ignored (Inspect) Visit Preview Jun 26, 2024 8:25pm

@changeset-bot

changeset-bot Bot commented Jun 10, 2024

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 02dbbbf

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@bigcommerce/catalyst-client Patch
@bigcommerce/catalyst-core Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Comment thread packages/client/src/client.ts Outdated
if (!this.config.channelId || this.config.channelId === '1') {
private getEndpoint(channelId?: string) {
if (channelId && channelId !== '1') {
return `https://store-${this.config.storeHash}-${channelId}.${graphqlApiDomain}/graphql`;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We've backfilled all/most default channels with the channel-specific canonical URL, so we should be able to simplify this to https://store-${this.config.storeHash}-${channelId ?? this.config.channelId}.${graphqlApiDomain}/graphql and enforce channel ID to be set.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good info, wasn't aware of this.

@github-actions

Copy link
Copy Markdown
Contributor

⚡️🏠 Lighthouse report

Lighthouse ran against https://catalyst-latest-cmb9i8dwn-bigcommerce-platform.vercel.app

🖥️ Desktop

We ran Lighthouse against the changes on a desktop and produced this report. Here's the summary:

Category Score
🟠 Performance 82
🟢 Accessibility 100
🟢 Best practices 100
🟢 SEO 90

📱 Mobile

We ran Lighthouse against the changes on a mobile and produced this report. Here's the summary:

Category Score
🟢 Performance 94
🟢 Accessibility 100
🟢 Best practices 100
🟢 SEO 92

@jorgemoya
jorgemoya added this pull request to the merge queue Jun 27, 2024
Merged via the queue into main with commit 8766305 Jun 27, 2024
@jorgemoya
jorgemoya deleted the add-channelId-param-client branch June 27, 2024 19:15
@github-actions github-actions Bot mentioned this pull request Jun 26, 2024
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.

2 participants