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

refactor(components): lazy load custom view selector in modular tab components #3691

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

emmenko
Copy link
Member

@emmenko emmenko commented Jan 10, 2025

Main goals:

  • decouple loading of the Custom View selector when tabular component is used outside of MC (evaluate if window.app is defined)
  • implies also a decoupling of react-router (used by the custom view selector)

@emmenko emmenko requested a review from a team as a code owner January 10, 2025 12:08
Copy link

changeset-bot bot commented Jan 10, 2025

🦋 Changeset detected

Latest commit: 68d5b82

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

This PR includes changesets to release 36 packages
Name Type
@commercetools-frontend/application-components Patch
@commercetools-frontend/application-shell Patch
@commercetools-frontend/codemod Patch
@commercetools-frontend/mc-scripts Patch
@commercetools-frontend/react-notifications Patch
@commercetools-applications/merchant-center-template-starter-typescript Patch
@commercetools-applications/merchant-center-template-starter Patch
@commercetools-applications/merchant-center-custom-view-template-starter-typescript Patch
@commercetools-applications/merchant-center-custom-view-template-starter Patch
@commercetools-local/playground Patch
@commercetools-local/visual-testing-app Patch
@commercetools-website/components-playground Patch
@commercetools-frontend/cypress Patch
@commercetools-backend/eslint-config-node Patch
@commercetools-backend/express Patch
@commercetools-backend/loggers Patch
@commercetools-frontend/actions-global Patch
@commercetools-frontend/application-config Patch
@commercetools-frontend/application-shell-connectors Patch
@commercetools-frontend/assets Patch
@commercetools-frontend/babel-preset-mc-app Patch
@commercetools-frontend/browser-history Patch
@commercetools-frontend/constants Patch
@commercetools-frontend/create-mc-app Patch
@commercetools-frontend/eslint-config-mc-app Patch
@commercetools-frontend/i18n Patch
@commercetools-frontend/jest-preset-mc-app Patch
@commercetools-frontend/jest-stylelint-runner Patch
@commercetools-frontend/l10n Patch
@commercetools-frontend/mc-dev-authentication Patch
@commercetools-frontend/mc-html-template Patch
@commercetools-frontend/notifications Patch
@commercetools-frontend/permissions Patch
@commercetools-frontend/sdk Patch
@commercetools-frontend/sentry Patch
@commercetools-frontend/url-utils 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

Copy link

vercel bot commented Jan 10, 2025

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

Name Status Preview Comments Updated (UTC)
mc-app-kit-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 10, 2025 1:22pm
merchant-center-application-kit-components-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 10, 2025 1:22pm

Comment on lines +176 to +178
const { currentCustomViewLocatorCode } = useCustomViewLocatorSelector(
props.customViewLocatorCodes
);
Copy link
Member Author

Choose a reason for hiding this comment

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

We are now using the hook here to match the router location

Comment on lines +7 to +12
if (typeof window !== 'undefined' && typeof window.app !== 'undefined') {
return import(
'./custom-views-selector' /* webpackChunkName: "custom-views-selector" */
);
}
return Promise.resolve({ default: () => null });
Copy link
Member Author

Choose a reason for hiding this comment

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

Here we evaluate if the component should be lazy loaded or not, by relying on window.app being defined

Comment on lines +3 to +6
export type TCustomViewLocatorCodeLocationDescriptor = {
pathname?: string;
search?: string;
};
Copy link
Member Author

Choose a reason for hiding this comment

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

Instead of using the type LocationDescriptor from history, we define our own similar type.

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