Skip to content

Conversation

@brijeshb42
Copy link
Contributor

@brijeshb42 brijeshb42 commented May 30, 2024

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: #83
Fixes: #88

@brijeshb42 brijeshb42 requested a review from siriwatknp May 30, 2024 08:09
@brijeshb42 brijeshb42 added type: enhancement It’s an improvement, but we can’t make up our mind whether it's a bug fix or a new feature. integration: nextjs Next.js labels May 30, 2024
@brijeshb42 brijeshb42 force-pushed the nextjs-config-api branch from 4c0c715 to 6d90d26 Compare May 30, 2024 08:24
@mnajdova mnajdova added the breaking change Introduces changes that are not backward compatible. label Jun 17, 2024
Copy link
Member

@mnajdova mnajdova left a 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

@brijeshb42
Copy link
Contributor Author

brijeshb42 commented Jun 17, 2024

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.

@brijeshb42 brijeshb42 requested a review from mnajdova June 17, 2024 09:44
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged. label Jun 17, 2024
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
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged. label Jun 21, 2024
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged. label Jul 2, 2024
@siriwatknp
Copy link
Member

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. createPigmentPlugin:

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.

@brijeshb42
Copy link
Contributor Author

brijeshb42 commented Jul 11, 2024

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.

@brijeshb42 brijeshb42 closed this Jan 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking change Introduces changes that are not backward compatible. integration: nextjs Next.js PR: out-of-date The pull request has merge conflicts and can't be merged. type: enhancement It’s an improvement, but we can’t make up our mind whether it's a bug fix or a new feature.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[examples] Update Next.js example to use ESM How can I custom a theme with NX+Next.js?

3 participants