Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Sep 6, 2025

  • Implement generic metadata filtering for Makeswift runtime to prefer BigCommerce defaults
  • Fix ESLint violations in makeswift-page-shim.tsx:
    • Replace any types with proper TypeScript types (MetadataValue, MetadataObject)
    • Add proper JSDoc documentation with @param and @returns annotations
    • Remove continue statements and for...of loops (replaced with reduce)
    • Fix spacing and formatting issues
    • Eliminate unsafe type assignments and member access
    • Make code fully type-safe while preserving same functionality

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

@vercel
Copy link

vercel bot commented Sep 6, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
catalyst-b2b Ready Ready Preview Comment Sep 11, 2025 1:15am
catalyst-canary Ready Ready Preview Comment Sep 11, 2025 1:15am
3 Skipped Deployments
Project Deployment Preview Comments Updated (UTC)
catalyst Ignored Ignored Sep 11, 2025 1:15am
catalyst-au Ignored Ignored Sep 11, 2025 1:15am
catalyst-uk Ignored Ignored Sep 11, 2025 1:15am

@changeset-bot
Copy link

changeset-bot bot commented Sep 6, 2025

⚠️ No Changeset found

Latest commit: 3d7a389

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

Copilot AI changed the title [WIP] Do not use the favicon from the makeswift runtime, instead use the favicon from the favicon route. The makeswift runtime is overriding it in effect and that needs to be disabled; do some research on how this is working in @makeswift/makeswift and disab... Fix Makeswift runtime favicon override to use BigCommerce store favicon Sep 6, 2025
Copilot AI requested a review from bookernath September 6, 2025 15:39
Copilot finished work on behalf of bookernath September 6, 2025 15:39
@bookernath
Copy link
Contributor

@copilot can we come up with a generic solution to only use the metadata from the makeswift runtime if it's defined? I'd like it to only override the data from the BigCommerce Storefront GraphQL API if there are overrides available. So for the favicon - prefer the store favicon to the default makeswift favicon. For other meta properties like title and description - use them only if they're non-falsy (e.g. ignore an empty string, use the BC default) but use them if they're defined.

Copy link
Contributor Author

Copilot AI commented Sep 6, 2025

@copilot can we come up with a generic solution to only use the metadata from the makeswift runtime if it's defined? I'd like it to only override the data from the BigCommerce Storefront GraphQL API _...

I've implemented a generic metadata filtering solution that addresses your requirements. The new filterMetadata function:

For favicon: Always disabled (favicon: false) to prefer the store favicon over Makeswift defaults

For other metadata: Only includes properties with meaningful values:

  • Filters out empty strings, null, and undefined to use BigCommerce defaults
  • Preserves non-empty strings, numbers, booleans, and arrays
  • Recursively handles nested objects (like OpenGraph metadata)
  • Removes empty objects that would only contain favicon: false

Examples:

  • title: "" → filtered out (uses BC default)
  • title: "Custom Title" → included (overrides BC default)
  • description: null → filtered out (uses BC default)
  • keywords: ["tag1", "tag2"] → preserved as-is

This ensures Makeswift metadata only overrides BigCommerce when there are actual meaningful values to use. Commit: 97cbb42

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • googlechromelabs.github.io
    • Triggering command: node install.mjs (dns block)
  • https://storage.googleapis.com/chrome-for-testing-public/137.0.7151.55/linux64/chrome-headless-shell-linux64.zip
    • Triggering command: node install.mjs (http block)
  • https://storage.googleapis.com/chrome-for-testing-public/137.0.7151.55/linux64/chrome-linux64.zip
    • Triggering command: node install.mjs (http block)
  • https://storage.googleapis.com/chrome-for-testing-public/140.0.7339.80/linux64/chrome-linux64.zip
    • Triggering command: node install.mjs (http block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI changed the title Fix Makeswift runtime favicon override to use BigCommerce store favicon Implement generic metadata filtering for Makeswift runtime to prefer BigCommerce defaults Sep 6, 2025
Copilot finished work on behalf of bookernath September 6, 2025 15:50
@bookernath
Copy link
Contributor

@copilot fix the lint please Run pnpm run lint -- --max-warnings=0

@bigcommerce/catalyst@0.***.0 lint /home/runner/work/catalyst/catalyst
dotenv -e .env.local -- turbo lint -- --max-warnings=0

Attention:
Turborepo now collects completely anonymous telemetry regarding usage.
This information is used to shape the Turborepo roadmap and prioritize features.
You can learn more, including how to opt-out if you'd not like to participate in this anonymous program, by visiting the following URL:
https://turborepo.com/docs/telemetry

• Packages in scope: @bigcommerce/catalyst, @bigcommerce/catalyst-client, @bigcommerce/catalyst-makeswift, @bigcommerce/create-catalyst, @bigcommerce/eslint-config-catalyst
• Running lint in 5 packages
• Remote caching enabled
@bigcommerce/catalyst-client:build
@bigcommerce/catalyst-client:lint
@bigcommerce/create-catalyst:lint
@bigcommerce/catalyst:lint
m@bigcommerce/catalyst-makeswift:lint
cache miss, executing 4
*74a63745d3a86b

@bigcommerce/catalyst-makeswift@***.2.0 lint /home/runner/work/catalyst/catalyst/core
next lint --max-warnings=0

next lint is deprecated and will be removed in Next.js ***6.
For new projects, use create-next-app to choose your preferred linter.
For existing projects, migrate to the ESLint CLI:
npx @next/codemod@canary next-lint-to-eslint-cli .

Attention: Next.js now collects completely anonymous telemetry regarding usage.
This information is used to shape Next.js' roadmap and prioritize features.
You can learn more, including how to opt-out if you'd not like to participate in this anonymous program, by visiting the following URL:
https://nextjs.org/telemetry

./lib/makeswift/makeswift-page-shim.tsx
4: Error: Missing JSDoc @returns for function. valid-jsdoc
4: Error: Missing JSDoc for parameter 'metadata'. valid-jsdoc
20:35 Error: Unexpected any. Specify a different type. @typescript-eslint/no-explicit-any
20:4*** Error: Unexpected any. Specify a different type. @typescript-eslint/no-explicit-any
24:*** Warning: Delete ·· prettier/prettier
29:9 Error: Unexpected any. Specify a different type. @typescript-eslint/no-explicit-any
3
:3 Error: iterators/generators require regenerator-runtime, which is too heavyweight for this guide to allow them. Separately, loops should be avoided in favor of array iterations. no-restricted-syntax
3***:45 Error: Unsafe argument of type any assigned to a parameter of type { [s: string]: unknown; } | ArrayLike<unknown>. @typescript-eslint/no-unsafe-argument
34:7 Error: Unexpected use of continue statement. no-continue
39:7 Error: Unexpected use of continue statement. no-continue
44:7 Error: Unexpected use of continue statement. no-continue
49:***6 Error: Unsafe member access [key] on an any value. @typescript-eslint/no-unsafe-member-access
50:7 Error: Unexpected use of continue statement. no-continue
53:38 Error: Unnecessary conditional, the types have no overlap. @typescript-eslint/no-unnecessary-condition
55:3 Error: Unsafe assignment of an any value. @typescript-eslint/no-unsafe-assignment
56:7 Warning: Expected blank line before this statement. @stylistic/padding-line-between-statements
56:5
Error: Unsafe argument of type any assigned to a parameter of type {}. @typescript-eslint/no-unsafe-argument
58:9 Error: Unsafe assignment of an any value. @typescript-eslint/no-unsafe-assignment
58:8 Error: Unsafe member access [key] on an any value. @typescript-eslint/no-unsafe-member-access
60:7 Warning: Expected blank line before this statement. @stylistic/padding-line-between-statements
60:7 Error: Unexpected use of continue statement. no-continue
64:4 Error: Unsafe member access [key] on an any value. @typescript-eslint/no-unsafe-member-access
7
:9 Error: Unsafe assignment of an any value. @typescript-eslint/no-unsafe-assignment
72:
Warning: Delete ·· prettier/prettier
73:46 Error: Unsafe assignment of an any value. @typescript-eslint/no-unsafe-assignment

info - Need to disable some ESLint rules? Learn more here: https://nextjs.org/docs/app/api-reference/config/eslint#disabling-rules
 ELIFECYCLE  Command failed with exit code .
Error: command finished with error: command (/home/runner/work/catalyst/catalyst/core) /home/runner/setup-pnpm/node_modules/.bin/pnpm run lint --max-warnings=0 exited (
)
Error: @bigcommerce/catalyst-makeswift#lint: command (/home/runner/work/catalyst/catalyst/core) /home/runner/setup-pnpm/node_modules/.bin/pnpm run lint --max-warnings=0 exited (***)

Tasks: 4 successful, 5 total
Cached: 0 cached, 5 total
Time: ***m8.347s
Failed: @bigcommerce/catalyst-makeswift#lint

ERROR run failed: command exited (***)
 ELIFECYCLE  Command failed with exit code ***.
Error: Process completed with exit code ***.

Copy link
Contributor Author

Copilot AI commented Sep 8, 2025

@copilot fix the lint please Run pnpm run lint -- --max-warnings=0

@bigcommerce/catalyst@0.***.0 lint /home/runner/work/catalyst/catalyst
...

Fixed all ESLint violations in the filterMetadata function. Changes include:

  • Replaced any types with proper TypeScript types (MetadataValue, MetadataObject)
  • Added complete JSDoc documentation with @param and @returns annotations
  • Eliminated continue statements and for...of loops (replaced with reduce)
  • Fixed spacing and formatting issues
  • Made all type assignments safe and eliminated unsafe member access
  • Preserved the same functionality while making the code fully type-safe

All 25 specific ESLint errors from the original lint output should now be resolved. Commit: bcbd07c

Copilot finished work on behalf of bookernath September 8, 2025 03:55
Copilot AI and others added 3 commits September 10, 2025 20:13
- Replace simple favicon disabling with comprehensive metadata filtering
- Filter out falsy values (empty strings, null, undefined) to use BigCommerce defaults
- Always disable favicon to prefer store favicon from /favicon.ico route
- Preserve arrays and meaningful boolean values
- Recursively filter nested objects while avoiding empty objects
- Handle edge cases: metadata=false, undefined, or empty objects

Co-authored-by: bookernath <[email protected]>
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