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 next configuration as an option for custom servers #2058

Merged
merged 6 commits into from
May 31, 2017

Conversation

reel
Copy link
Contributor

@reel reel commented May 24, 2017

Instead of having a next.config.js file, we can dynamically add configuration from the custom server.

The use case would be to support many different webpack configuration with a minimist flag.

I can add an example if wanted.

@timneutkens timneutkens requested a review from arunoda May 25, 2017 16:30
Copy link
Contributor

@arunoda arunoda left a comment

Choose a reason for hiding this comment

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

The idea looks great to me.
Could you work on this change?

this.http = null
this.config = getConfig(this.dir)
this.config = getConfig(this.dir, conf)
Copy link
Contributor

Choose a reason for hiding this comment

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

If we've a config via the option, it shouldn't read from the filesystem. Change the code to reflect that.

And make sure to get default configurations. May be we need to add method called withDefaults to server/config.js

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, for my use case, I still want to be able to read from filesystem to overwrite default & custom server.

That's why I have this order in Object.assign

Your thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@arunoda Is this OK?

@arunoda
Copy link
Contributor

arunoda commented May 31, 2017

@reel everything looks great to me. Let's wait for tests.

@arunoda
Copy link
Contributor

arunoda commented May 31, 2017

Actually the error is not our fault.
See: vercel/styled-jsx#217

So. I'm taking this in.

@arunoda arunoda merged commit 442c611 into vercel:master May 31, 2017
@reel reel deleted the custom-server-conf branch May 31, 2017 10:29
@reel
Copy link
Contributor Author

reel commented Jun 3, 2017

Thank's @arunoda

Here's a peak at what we can do now: https://github.com/usehenri/henri/blob/master/packages/view/conf.js

xuezhma pushed a commit to xuezhma/next.js that referenced this pull request Jun 22, 2017
* Add a configuration parameter to custom server startup

* Adding related documentation

* Do not access filesystem if configuration is supplied

* Make the configuration log clearer

* Make the conf default value to `null`
@lock lock bot locked as resolved and limited conversation to collaborators Jun 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants