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.

Sign up for GitHub

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

Jump to bottom

Feature/UI lib #206

Merged
merged 13 commits into from
Oct 25, 2023
Merged

Feature/UI lib #206

merged 13 commits into from
Oct 25, 2023

Conversation

nlkluth
Copy link
Member

@nlkluth nlkluth commented Oct 23, 2023 •
edited
Loading

What

Creates ui library for sharing components

Why

This project is being upgraded to allow developers to quickly spin up an E-commerce frontend with prebuilt components, so that they can focus their efforts on evaluating partner tools and technology

TODO

Future tasks to follow in subsequent PRs

  • Move remaining components
  • Clean up duplicated dependencies and components from ui and nextjs projects
  • Any additional "componentization" needed to create easily reusable chunks
  • Decouple any direct Sanity relationships (most likely types)

@vercel
Copy link

vercel bot commented Oct 23, 2023 •
edited
Loading

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

Name Status Preview Comments Updated (UTC)
nextjs-sanity-fe ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 24, 2023 9:31pm

nlkluth
nlkluth commented Oct 23, 2023
View reviewed changes
package.json
@@ -26,6 +26,19 @@
"test:e2e-mock:start": "start-server-and-test next:dev:mock http://localhost:3000 test:e2e-mock"
},
"devDependencies": {
"@storybook/addon-essentials": "^7.4.0",
Copy link
Member Author

@nlkluth nlkluth Oct 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Added shared dependencies for now as I slowly move files over. These might be able to be cleaned up when the migration is done

packages/nextjs/components/Header/CartContent.tsx Outdated
@@ -1,7 +1,7 @@
import * as React from "react";
import { IoIosClose } from "react-icons/io";
import Link from "next/link";
import { Button } from "components/Button";
import { Button } from "ui";
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 is the result

packages/nextjs/package.json Outdated Show resolved Hide resolved
packages/tests-e2e/package.json Show resolved Hide resolved
packages/ui/.storybook/decorators/TestHarness.tsx Outdated Show resolved Hide resolved
@nlkluth
Copy link
Member Author

nlkluth commented Oct 23, 2023

Looking into CI errors 👀

@carloskelly13
Copy link
Contributor

carloskelly13 commented Oct 23, 2023 •
edited
Loading

This looks great, but I see a couple issues currently with getting the UI library to build (and get consumed in the main app). I wonder if we should look at using tsup or perhaps Vite for building and creating a first-class library with ESM output. An example here on Nuka → https://github.com/FormidableLabs/nuka-carousel/blob/main/packages/nuka/tsup.config.ts

Additionally we can then specify in the package.json of the ui library which outputs to use when imported

https://github.com/FormidableLabs/nuka-carousel/blob/main/packages/nuka/package.json#L5-L7

Finally when consumed, we can reference it using a * in the main app (https://github.com/FormidableLabs/nuka-carousel/blob/main/website/package.json#L24)
then import as normal and we get the correct types and such:
https://github.com/FormidableLabs/nuka-carousel/blob/main/website/src/components/demos.tsx#L2

@nlkluth
Copy link
Member Author

nlkluth commented Oct 23, 2023

@carloskelly13 Thanks for that info. I was definitely unsure how to get to that step so I appreciate it! I'll look into this 👍

scottrippey
scottrippey reviewed Oct 24, 2023
View reviewed changes
packages/ui/package.json Outdated Show resolved Hide resolved
packages/ui/tsconfig.json Outdated Show resolved Hide resolved
scottrippey
scottrippey reviewed Oct 24, 2023
View reviewed changes
Copy link
Member

@scottrippey scottrippey left a comment

Choose a reason for hiding this comment

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

Overall looks great! The first step toward some great improvements and separation.

scottrippey
scottrippey reviewed Oct 24, 2023
View reviewed changes
tsconfig.json Outdated Show resolved Hide resolved
@nlkluth
Copy link
Member Author

nlkluth commented Oct 24, 2023 •
edited
Loading

Build worked 🎉 However, looking into UI regression 👀 Edit: fixed

@nlkluth nlkluth merged commit a8f48d1 into main Oct 25, 2023
7 of 8 checks passed
@nlkluth nlkluth deleted the feature/ui-lib branch October 25, 2023 19:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@scottrippey scottrippey scottrippey approved these changes

@colinrobertbrooks colinrobertbrooks colinrobertbrooks approved these changes

@carloskelly13 carloskelly13 Awaiting requested review from carloskelly13

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@nlkluth @carloskelly13 @scottrippey @colinrobertbrooks