cli/lib/merge & setup ui mantine

[nickpdemarco]

Reviewer's Guide

Key files

• packages/cli/src/lib/merge/index.js Generic AST merging algorithm. Or, read the docs, or read the tests.
• packages/cli/src/lib/merge/strategy.js Named higher-order merge strategies like concatUnique.
• packages/cli/src/lib/extendFile.js New utility for modifying App.js|tsx etc.

Straightforward changes

packages/cli/src/commands/setup/ui/libraries/mantine.js Essentially a copy of chakra's approach.
packages/cli/src/commands/setup/ui/libraries/chakra-ui.js Tweaks to use new internal apis.
packages/cli/src/lib/configureStorybook.js Now, just calls to fs and merge.

Description

cli/lib/merge

This PR introduces a new merge sublibrary of cli/lib, designed to facilitate the merging of Javascript files. See the README for more complete documentation.

The merge implementation requires @babel/generator and @types/babel__generator, and both have been added to the framework-level package.json.

Please note: I consider this code to be 0.1 quality - I've really only provided tests to prove it works for mostly-reasonable, small javascript files. Much more testing is required before using it in contexts outside of Storybook configuration merges. So, think of it as a foundation, rather than a perfectly complete, generic Javascript AST merger.

rw setup *

This merge algorithm is used in service of rw setup i18n, rw setup ui chakra-ui, and a newly-added rw setup ui mantine, each of which need to extend our storybook configuration file, which lives at web/config in Redwood Projects.

To accomodate this, we've slightly changed how we handle storybook configurations. First, we have a "base" configuration, which provides some useful examples and comments, but is a no-op from the perspective of Storybook. In the framework, the file lives at cli/src/lib/templates. If a user never invokes a Redwood command that requires a storybook configuration, they'll never see this file. However, if they do, said file will be used as the "base" into which command-specific extensions (e.g. those needed by i18n) will be merged.

This notion of "base" and "extension" occurs often in the merge code, and imply a handful of design decisions. Code in the base is considered sacred (since it may be user-generated) and is never deleted, overwritten, or otherwise manipulated in a lossy manner. As an example, leading comments on semantically-equivalent nodes will prefer the base version. Assuming we're using a concatUnique merge strategy for ArrayExpression, the following demonstrates a simple merge.

/* base.js */
// my favorite variable
const x = [1]

/* ext.js */
// a tolerable variable
const x = [2]

/* merged.js */
// my favorite variable
const x = [1, 2]

extendJSXFile

See extendFile.

One of the earlier problems addressed by this ever-expansive PR was the manipulation of App.{js|tsx} during various rw setup commands. I still believe the implementation provided in this PR is an improvement over mainline, but I am beginning to wonder if extendFile.js could also be improved by some Babel-ification. In the interest of preventing yet-further scope creep on this PR, I leave that as a task for another time.

TODO

I believe the code in this PR is now ready for proper review. It has grown considerably in scope from its original mission (to simply add Mantine as another UI provider), and as such it may be appropriate to split the merge code into another PR. If any of the reviewers here deem that appropraite, I can make the change this weekend.

Furthermore, I need to enrich the documentation in the source tree, as well as on the public-facing websites. I figure that shouldn't block a review, so I'm taking this PR out of draft.

• Richer merge documentation with details on merge strategies, and deeper explanation of the overall approach.
• Short recorded screencast with overview of PR
• "Design Library Configuration" doc, with link added to <lib>.config.js
• CLI Commands Doc

[netlify]

✅ Deploy Preview for redwoodjs-docs ready!

Name	Link
🔨 Latest commit	b9349a8
🔍 Latest deploy log	https://app.netlify.com/sites/redwoodjs-docs/deploys/62f0e9ef2ed45300088066c1
😎 Deploy Preview	https://deploy-preview-4900--redwoodjs-docs.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site settings.

[Tobbe]

This is a great start @nickpdemarco! Thanks for taking the time to work on this. One of the more difficult parts when adding new styling frameworks seems to be to get the StoryBook integration working. So please make sure give that an extra round of testing.

[nickpdemarco]

Thanks for the encouragement @Tobbe! I'll make a point to stress test the storybook integrations.

[nickpdemarco]

One of the more difficult parts when adding new styling frameworks seems to be to get the StoryBook integration working. So please make sure give that an extra round of testing.

@Tobbe With this last push, I think I've replicated chakra-ui's storybook integration. I'm creating a web/config/storybook.preview.js that wraps the story function in a MantineProvider, which is all chakra's implementation appears to do.

In writing that, though, I realized that we'll be wrapping the app component in a different instance of [Mantine|Chakra]Provider than the one created in App.[js,tsx], and that means we won't share any theme overrides between the app context and the storybook context. That strikes me as a big value loss for debugging components built with chakra/mantine in storybook. Probably a non-starter for me personally.

If I understand the situation correctly, this means we need some sort of standard location / file for theme overrides, so both App.[js|tsx] and storybook.preview.js can point to it. That seems like a medium-sized design decision for Redwood generally so I wanted to get your thoughts on it before proceeding.

[Tobbe]

@nickpdemarco Thanks for the summary of the problem. I agree that we should try to find a better solution. @virtuoushub Is this something you've been thinking about when working with RW's Storybook integration?

[nickpdemarco]

A strawman for your consideration: I have something working reasonably well:

When we run yarn rw setup ui maintine, I add a src/themes directory and create src/themes/mantine.config.js with a default export of an empty object.

Then I can reference that in both App and the storybook preview as follows:

import * as theme from 'src/themes/mantine.config'
const extendedTheme = extendTheme(theme)

Does nothing by default, of course, but gives the user a reasonable place to put their theme overrides, and automatically gets changes visible in Storybook and in the app. I think Chakra would work similarly, if we decided to go this route.

[thedavidprice]

If I understand the situation correctly, this means we need some sort of standard location / file for theme overrides, so both App.[js|tsx] and storybook.preview.js can point to it. That seems like a medium-sized design decision for Redwood generally so I wanted to get your thoughts on it before proceeding.

Great question and, yes, we want to make changes here per recommendations from the Storybook team. @Tobbe has already looped in @virtuoushub who can help sync things up.

[Tobbe]

I'm going to be honest here and say I hadn't actually heard of mantine until you created this PR. And now I found https://ui.mantine.dev and I'm really impressed! This is going to be a great addition to RW 🚀

[virtuoushub]

@nickpdemarco Thanks for the summary of the problem. I agree that we should try to find a better solution. @virtuoushub Is this something you've been thinking about when working with RW's Storybook integration?

@Tobbe, yes, a bit. I quite like @nickpdemarco's approach:

A strawman for your consideration: I have something working reasonably well:

When we run yarn rw setup ui maintine, I add a src/themes directory and create src/themes/mantine.config.js with a default export of an empty object.

Then I can reference that in both App and the storybook preview as follows:

import * as theme from 'src/themes/mantine.config'
const extendedTheme = extendTheme(theme)

Does nothing by default, of course, but gives the user a reasonable place to put their theme overrides, and automatically gets changes visible in Storybook and in the app. I think Chakra would work similarly, if we decided to go this route.

@nickpdemarco thanks for this PR! 🔥 🔥 🔥

I have reached out to some of the folks who maintain Storybook to see what they recommend for things like this.

---

Depending on how common of an interface we can get around things like extending themes, it might make sense to adopt a pattern that is framework agnostic.

Rough idea is some template such as:

import * as theme from `src/themes/${framework}.config`
const extendedTheme = extendTheme(theme)

[nickpdemarco]

@virtuoushub I hear you - I think a generalized theme/library interface is plausible. Bear in mind that In this particular case, I think there's an unusual amount of API symmetry between Chakra and Mantine (Mantine took a lot of inspiration from Chakra, if I have my history right)

I'm also realizing that the current tailwind theme configuration files end up in web/config. I'm still a fan of putting theme configuration files in web/themes, but not enough to violate the precedent tailwind set. On the other hand, when I think "I want to tweak my theme" I don't think "config".

For this PR, to keep the surface area small and to be a good consistent citizen, I figure I'll move the mantine and chakra generated theme files to web/config as well, but I'd love to hear people's thoughts about where these files should end up in the long term.

[thedavidprice]

@nickpdemarco @Tobbe @jtoar
Just 🔥🚀

Well done!

[nomadme]

Using mantine breaks tests.

tests are failing now because mantine requires the render function to include the 'mantineprovider' in the path.

"@mantine/core: MantineProvider was not found in component tree, make sure you have it in your app"

@nickpdemarco How should I handle the failing tests?