-
-
Notifications
You must be signed in to change notification settings - Fork 32.4k
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
[pigment-css][example] Add example project with Remix #41700
Conversation
Netlify deploy previewhttps://deploy-preview-41700--material-ui.netlify.app/ Bundle size report |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's revert the change to apps/pnpm-lock.yml, it's not related
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This reminds that we need to update all examples to use the next tag. I will handle this in a separate PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think Remix should be added to apps/pigment-css-remix-app
too, it can be the same PR or a separate one, your call!
17ad79d
to
cd9f9b4
Compare
Agree! I will leave few questions here, even tough they are not 100% related to the PR:
|
This is already the case and part of the current CI build. Both vite and next.js apps are built. The previous propTypes issue was failing in dev environment. And in most cases, the actual fix lies in the package code and rarely in the app code. So it should get autofixed since we always tag the latest release.
The example app is very minimal and doesn't cover complex use-case which is part of our current |
IMO, we can just convert the Vite app to a remix one. It'll provide coverage for both and we won't need to use the custom vite plugin for routing as Remix will already handle that. |
Nice! |
7696161
to
c80e1a9
Compare
c80e1a9
to
81066fd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
#39765