feat(v2): update swizzle command to suggest component/theme

[anshulrgoyal]

Motivation

Better user experience for swizzle cli. Based on #2984

Have you read the Contributing Guidelines on pull requests?

yes

Test Plan

[docusaurus-bot]

Deploy preview for docusaurus-2 ready!

Built with commit 3aad5f3

https://deploy-preview-3021--docusaurus-2.netlify.app

[slorber]

That looks like a nice improvement.

What about making the message clearer like "this is the list of available themes you can swizzle" or something?

[anshulrgoyal]

That looks like a nice improvement.

What about making the message clearer like "this is the list of available themes you can swizzle" or something?

Something like this?

[slorber]

yes, that looks fine thanks

[slorber]

Thanks @anshulrgoyal that looks like a first version.

Here are a few suggestions to improve:

• When component/theme not found, and no suggestion, show again the list of available components
• Show components to swizzle in subfolders too (for example, it's a bit weird to suggest hooks, user is more likely to swizzle a single hook instead of the whole folder, and the following is actually working: yarn swizzle @docusaurus/theme-classic hooks/useAnnouncementBar)

[slorber]

I wonder if also the theme should not explicitly define the component list to swizzle.

Because currently, some theme components are considered "implementation details", and we don't necessarily want to encourage the users to swizzle those, as this would implicitly expand the api surface of a theme.

What about exploring the idea of defining, at the theme level, a list of swizzle suggestions?

We could allow the user to swizzle values out of the suggestions, with a flag like --danger or something, so that they know that they are swizzling internal comp, and they may have to handle breaking changes

[anshulrgoyal]

Thanks @anshulrgoyal that looks like a first version.

Here are a few suggestions to improve:

* When component/theme not found, and no suggestion, show again the list of available components

* Show components to swizzle in subfolders too (for example, it's a bit weird to suggest `hooks`, user is more likely to swizzle a single hook instead of the whole folder, and the following is actually working: `yarn swizzle @docusaurus/theme-classic hooks/useAnnouncementBar`)

I will add a tree-like listing for components and idea of theme defining swizzle component is nice but It would be a breaking change :), we can let user export a list of swizzle component.

[anshulrgoyal]

Or we can let the theme define a list of dangerous component

[anshulrgoyal]

Something like this works @slorber?

[anshulrgoyal]

For component not found:

If no suggestion:

[slorber]

thanks that looks better

I will add a tree-like listing for components and idea of theme defining swizzle component is nice but It would be a breaking change :), we can let user export a list of swizzle component.

Not sure what you mean by breaking changes? it's just a list of suggestions displayed, it should not forbid the user to swizzle a component that is internal, just not suggest to swizzle it. This feature should not affect current swizzle behavior. Anyway even if we did forbid the user to swizzle with the cli, he would still be able to copy the files manually if he really wants to, and that would work.

Or we can let the theme define a list of dangerous component

I'd rather have the opposite instead. By default comps are considered as internal code, unless we explicitly add them to the list, is less risky than having them being recommended for swizzling by default. Devs will likely forget to add comps to that list. If this happens, it's not a big deal, the comp won't be suggested in the list but the swizzle command on this comp will still work anyway.

[slorber]

Maybe we should add a method like isSwizzleRecommendation(compName: string): boolean or something, and let the theme decide if it wants to do a allow/denylist or whatever?

I think the classic theme should rather be an allow list to avoid recommending to swizzle impl details, but for other smaller themes we might as well want to suggest all comps by default, and avoid annoying the user to manually craft a list of allowed comps to swizzle

[anshulrgoyal]

thanks that looks better

I will add a tree-like listing for components and idea of theme defining swizzle component is nice but It would be a breaking change :), we can let user export a list of swizzle component.

Not sure what you mean by breaking changes? it's just a list of suggestions displayed, it should not forbid the user to swizzle a component that is internal, just not suggest to swizzle it. This feature should not affect current swizzle behavior. Anyway even if we did forbid the user to swizzle with the cli, he would still be able to copy the files manually if he really wants to, and that would work.

Or we can let the theme define a list of dangerous component

I'd rather have the opposite instead. By default comps are considered as internal code, unless we explicitly add them to the list, is less risky than having them being recommended for swizzling by default. Devs will likely forget to add comps to that list. If this happens, it's not a big deal, the comp won't be suggested in the list but the swizzle command on this comp will still work anyway.

I thought u meant to not allow swizzle of certain component in a theme.

[anshulrgoyal]

Maybe we should add a method like isSwizzleRecommendation(compName: string): boolean or something, and let the theme decide if it wants to do a allow/denylist or whatever?

I think the classic theme should rather be an allow list to avoid recommending to swizzle impl details, but for other smaller themes we might as well want to suggest all comps by default, and avoid annoying the user to manually craft a list of allowed comps to swizzle

I am more inclined to list of allowed component because it will give more control to @docusaurus/core which can handle it in a more uniform method. If we go with isSwizzleRecommendation we have to call it with every component to generate a list.

[anshulrgoyal]

@slorber something like this?

[anshulrgoyal]

const components=[  // 'AnnouncementBar',
  // 'BlogListPage',
  // 'BlogListPaginator',
  // 'BlogPostItem',
  // 'BlogPostPage',
  // 'BlogPostPaginator',
  // 'BlogTagsListPage',
  // 'BlogTagsPostsPage',
  'CodeBlock',
  // 'DocItem',
  // 'DocPage',
  // 'DocPaginator',
  'DocSidebar',
  'Footer',
  // 'Heading',
  // 'Layout',
  // 'MDXComponents',
  // 'Navbar',
  'NotFound',
  'SearchBar',
  // 'TabItem',
  // 'Tabs',
  // 'ThemeContext',
  // 'ThemeProvider',
  // 'Toggle',
  // 'UserPreferencesContext',
  // 'UserPreferencesProvider',
  // 'hooks/useAnnouncementBar',
  // 'hooks/useHideableNavbar',
  // 'hooks/useLocationHash',
  // 'hooks/useLockBodyScroll',
  // 'hooks/useLogo',
  // 'hooks/usePrismTheme',
  // 'hooks/useScrollPosition',
  // 'hooks/useTOCHighlight',
  // 'hooks/useTabGroupChoice',
  'hooks/useTheme',
  // 'hooks/useThemeContext',
  // 'hooks/useUserPreferencesContext',
  // 'hooks/useWindowSize',
  'prism-include-languages',]

[slorber]

That looks good!

Will test this soon and merge it for the next release

[teikjun]

@anshulrgoyal btw, there's a merge conflict here

[anshulrgoyal]

@anshulrgoyal btw, there's a merge conflict here

Thanks. I will resolve them

[slorber]

that looks good thanks