-
-
Notifications
You must be signed in to change notification settings - Fork 38
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
[nextjs] Update the withPigment API to accept config #114
base: master
Are you sure you want to change the base?
Conversation
4c0c715
to
6d90d26
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.
I am 👌 with this change, we just have to make sure we document it as a breaking change in the changelog
6d90d26
to
e009961
Compare
I'd say it deprecates the current API as it can still take the input as second argument. But I'll update the docs to note this. |
e009961
to
e77eff4
Compare
through `pigment` key from the nextConfig object itself instead of as a 2nd argument to the call. This is a standard approach followed by other nextjs plugins Fixes: mui#83
e77eff4
to
53605f1
Compare
I am dropping my thought here based on my user expectation and writing docs. I would like all bundler plugin to use the same name, e.g. Next.js import { createPigmentPlugin } from '@pigment-css/nextjs-plugin';
const pigmentPlugin = createPigmentPlugin(…pigment config);
export default pigmentPlugin(…next config) Vite: import { createPigmentPlugin } from '@pigment-css/vite-plugin';
const pigmentPlugin = createPigmentPlugin(…pigment config);
export default defineConfig({
plugins: [pigmentPlugin()]
}) With the above signature, writing docs will be a lot easier because we can refer to the setup once and later refer to only "pigment config" for other feature. |
Seems like in latest versions of Next.js, it warns users when it finds an unknown key in the config ⚠ Invalid next.config.mjs options detected:
⚠ Unrecognized key(s) in object: 'pigment'
⚠ See more info here: https://nextjs.org/docs/messages/invalid-next-config Now, I am not so sure that we should do this. |
through
pigment
key from the nextConfig object itself instead of as a 2nd argument to the call. This is a standard approach followed by other nextjs pluginsFixes: #83
Fixes: #88