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.

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

Add defineConfig function to next/config #67963

Open
wants to merge 4 commits into
base: canary
Choose a base branch
from

Conversation

AntoineKM
Copy link

@AntoineKM AntoineKM commented Jul 19, 2024

What did I change?

I added a defineConfig function in runtime-config.external.ts.
But to be honest I think it would make more sense if it was in config-shared.ts the problem is that you would use

// next.config.ts
import { defineConfig } from “next/server"

instead of

// next.config.ts
import { defineConfig } from “next/config”

like vuejs, vitejs, solidjs, cypress or others do.

Why did I change it?

Fixes: https://x.com/nurodev/status/1813945402049388893

Preview and Usage
image

define-config.mp4

@AntoineKM AntoineKM marked this pull request as draft July 19, 2024 17:40
@ijjk
Copy link
Member

ijjk commented Jul 19, 2024

Allow CI Workflow Run

  • approve CI run for commit: 696ab0b

Note: this should only be enabled once the PR is ready to go and can only be enabled by a maintainer

@AntoineKM AntoineKM changed the title Update runtime-config.external.ts to include defineConfig function Add defineConfig function to next/config Jul 19, 2024
@ijjk ijjk added tests Documentation Related to Next.js' official documentation. labels Jul 19, 2024
@AntoineKM AntoineKM marked this pull request as ready for review July 19, 2024 20:36
@ijjk ijjk added the create-next-app Related to our CLI tool for quickly starting a new Next.js application. label Jul 19, 2024
Copy link
Author

@AntoineKM AntoineKM left a comment

Choose a reason for hiding this comment

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

I did a review everything looks ok, let me know if you see anything you don't like, I'll correct it.

Copy link
Member

@leerob leerob left a comment

Choose a reason for hiding this comment

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

I have some reservations for adding a new API to the overall surface area now that next.config.ts exists. I could probably be convinced, but at least for this PR, we definitely shouldn't update every place in the docs to use the new API until it's a bit more tested (regardless).

@AntoineKM
Copy link
Author

Hi @leerob, thanks for the feedback, so maybe the ideal would be to keep the current documentation and add a part in the documentation that explains how to use it and specifies that it's experimental?

@AntoineKM
Copy link
Author

And also, do we keep it in the templates or add it when it's really been tested and approved?

@AntoineKM
Copy link
Author

I've removed all the changes I've made to the documentation, because with hindsight, it will be preferable to integrate it gradually, depending on whether or not the community adopts this feature.
So I've only added to the documentation how it's used, I've also specified that it's experimental.

@AntoineKM AntoineKM requested a review from leerob July 21, 2024 16:46
@imranbarbhuiya
Copy link
Contributor

Even tho it's used in all the popular frameworks but what's the usecase of it when u can import the type and use it? Using a fn adds some runtime processing too and using ts type, you are still getting all the autocompletes

@AntoineKM
Copy link
Author

Even tho it's used in all the popular frameworks but what's the usecase of it when u can import the type and use it? Using a fn adds some runtime processing too and using ts type, you are still getting all the autocompletes

Hey @imranbarbhuiya, I would say:

  • Consistency across the ecosystem: Adopting this model in Next.js helps maintain consistency between frameworks.
  • Code cleanliness: There's no need to explicitly annotate types everywhere (especially since new developers will probably have trouble understanding what they are), reducing boilerplate and making the configuration file easier to read/maintain.

Important

Keep in mind that this isn't the game changer, it should be seen as a help for newcomers and/or a way of aligning with competing frameworks and their users migrating to/learning Next.js.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
create-next-app Related to our CLI tool for quickly starting a new Next.js application. Documentation Related to Next.js' official documentation. tests type: next
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants