Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

support $PORT environment var in next start directly without passing it as argument #10338

Closed
macrozone opened this issue Jan 30, 2020 · 11 comments · Fixed by #26066 · May be fixed by #44392
Closed

support $PORT environment var in next start directly without passing it as argument #10338

macrozone opened this issue Jan 30, 2020 · 11 comments · Fixed by #26066 · May be fixed by #44392

Comments

@macrozone
Copy link

macrozone commented Jan 30, 2020

Feature request

Is your feature request related to a problem? Please describe.

We start to spawn more and more nextjs projects and we have ci/cd which allows us to deploy a new project with absolut minimal effort by using some conventions. E.g. we want the apps to run on production using the start script in package.json. So every node app should just invoke npm run start.

Many frameworks use some conventions which makes this approach easy, e.g. using $PORT as the default port to listen to if its defined. Unfortunatly, nextjs does not respect this environment variable. So we have to modify newly created nextjs projects and adjust the package.json.start script with: next start --port $PORT

Describe the solution you'd like

next start without arguments start the server listening to env var $PORT if its defined, otherways falls back to 3000. --port argument would still have precedence

Describe alternatives you've considered

we could do more adjustments on our side, but supporting $PORT in nextjs would be a tiny change:

in https://github.com/zeit/next.js/blob/canary/packages/next/cli/next-start.ts#L48

const port = args['--port'] || process.env.PORT ||  3000;

Additional context

While this adjustment is extremly easy to do, it could have some impact on environments where PORT is already unexpectedly defined and no --port argument is used. i think this is a rare edge case. Most people who self host nextjs will have --port set, so it would not accidentially break for them.

@SkeLLLa
Copy link

SkeLLLa commented Mar 26, 2020

This will also is important for dockerized environments. If you use docker there's no easy way to use env variables in CMD directive. And production builds usually come without npm where you can pass that as part of script.

@danawoodman
Copy link

I was quite surprised that the documented .env/.env.test file PORT settings are not respected by the command line. This seems a bit unintuitive/non-standard and would be great if it could get addressed to make the package.json file simpler and make it more configurable using .env* vars

@timneutkens
Copy link
Member

This seems a bit unintuitive/non-standard

If you're going to make the argument for "non-standard" it's always good to provide many examples of where this is the case.

@danawoodman
Copy link

Sure. Below are some examples. I'll also add that pretty much every Node project I've interacted with in my professional career handles process.env.PORT. I don't think I'm too far off base making the claim supporting that is standard?

Sails.js allows setting PORT env to configure port:
https://sailsjs.com/documentation/concepts/configuration

Feathers allows setting env vars and then passing them into your app without flags:
https://docs.feathersjs.com/api/configuration.html#example

Keystone.js supports process.env.PORT:
https://v4.keystonejs.com/documentation/configuration/server-options/

Kraken.js allows PORT env:
krakenjs/kraken-js#142

Adonis.JS:
https://adonisjs.com/docs/3.2/env

Nest.js appears to also support it without flag:
https://docs.nestjs.com/techniques/configuration

@nfroidure
Copy link
Contributor

I would really like this to be done for $HOSTNAME and $PORT to respect good practices of Docker and Node described here : https://github.com/goldbergyoni/nodebestpractices/blob/master/sections/docker/bootstrap-using-node.md

The fact is that using CMD do not allow to have dynamic arguments : https://stackoverflow.com/questions/40454470/how-can-i-use-a-variable-inside-a-dockerfile-cmd

The following do not work:

ARG HOSTNAME=0.0.0.0
ENV HOSTNAME=$HOSTNAME
ARG PORT=8080
ENV PORT=$PORT

EXPOSE ${PORT}

CMD ["/usr/local/bin/node", "node_modules/next/dist/bin/next", "--", "start", "-p", ${PORT}, "-H", ${HOSTNAME}]

Allowing to use the env vars would help to reduce the command to:

CMD ["/usr/local/bin/node", "node_modules/next/dist/bin/next", "--", "start"]

In the meanwhile, I'm stuck with npm start 🤷.

@baba43
Copy link

baba43 commented Sep 2, 2020

I think this is also an issue when trying to deploy Next.js to Plesk environments.

The official Node.js extension allows us to execute an npm command. However, I've no clue how to tell Next.js to use the port specified as the environment variable when using next start.

@nfroidure
Copy link
Contributor

nfroidure commented Sep 15, 2020

While I think the issue is still relevant, just in case you missed it like me, there is a clean workaround here to use in the meanwhile: #11408 (comment)

I just adapted it that way to also pick HOSTNAME in the env:

/* eslint-disable */

const cli = require('next/dist/cli/next-start');

cli.nextStart([
  '-p', process.env.PORT || 3000,
  '-H', process.env.HOSTNAME || '0.0.0.0',
]);

That way it works well with Docker.

@desaidarshit

This comment has been minimized.

@danawoodman
Copy link

@timneutkens I've offered a list of other frameworks that do this, any chance this could be considered as an enhancement to Next? I've found my own work arounds but it would be swell if it could be addressed at some point 🍻

@timneutkens
Copy link
Member

See #11408 (comment). It's breaking so can only be added in a major version.

@nickwesselman
Copy link

ASP.NET Core does this very well IMHO, using an environment variable to specify bindings.

ASPNETCORE_URLS="http://localhost:5001;https://localhost:5002"

https://andrewlock.net/5-ways-to-set-the-urls-for-an-aspnetcore-app/

@vercel vercel locked and limited conversation to collaborators Apr 12, 2021
kodiakhq bot pushed a commit that referenced this issue Jun 14, 2021
- Enables excludeDefaultMomentLocales by default
- Adds distDir cleaning (See RFC #6009)
- Adds support for `PORT`
- Removes `router.events` from the server-side router as it should not be used server-side (long-standing todo that is potentially breaking). Note that it's still available as `Router.events` (import Router from 'next/router') and with `useRouter` in `useEffect`. Using it with `useEffect` is the correct way and I've updated the upgrading guide to reflect that
- Added webpack 5 to the upgrading guide
- Removed `Head.rewind` as it's been a no-op since Next.js 9.5 and can now be safely removed from user code

Fixes #11408 
Fixes #10338
Fixes #5554



## Bug

- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added

## Feature

- [x] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
- [x] Related issues linked using `fixes #number`
- [x] Integration tests added
- [x] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.

## Documentation / Examples

- [ ] Make sure the linting passes
flybayer pushed a commit to blitz-js/next.js that referenced this issue Jun 16, 2021
- Enables excludeDefaultMomentLocales by default
- Adds distDir cleaning (See RFC vercel#6009)
- Adds support for `PORT`
- Removes `router.events` from the server-side router as it should not be used server-side (long-standing todo that is potentially breaking). Note that it's still available as `Router.events` (import Router from 'next/router') and with `useRouter` in `useEffect`. Using it with `useEffect` is the correct way and I've updated the upgrading guide to reflect that
- Added webpack 5 to the upgrading guide
- Removed `Head.rewind` as it's been a no-op since Next.js 9.5 and can now be safely removed from user code

Fixes vercel#11408 
Fixes vercel#10338
Fixes vercel#5554



## Bug

- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added

## Feature

- [x] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
- [x] Related issues linked using `fixes #number`
- [x] Integration tests added
- [x] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.

## Documentation / Examples

- [ ] Make sure the linting passes

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
None yet
Projects
None yet
8 participants