ProteGO-Safe / web

Source code for ProteGO Safe PWA
https://safesafe.app
GNU General Public License v3.0
25 stars 14 forks source link

Start script broken on Windows #28

Closed piotr-cz closed 4 years ago

piotr-cz commented 4 years ago

Describe the bug The start script is broken on Windows OS

To Reproduce Steps to reproduce the behavior:

  1. Use dev machine with Windows OS
  2. Run yarn start/ npm start
  3. See error
    yarn run v1.22.5
    $ PORT=3001 react-scripts start
    'PORT' is not recognized as an internal or external command,
    operable program or batch file.
    error Command failed with exit code 1.
    info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

Expected behavior Development server starts at port 3001

Screenshots none provided

Desktop (please complete the following information):

Additional context PORT is one of the environment variables defined in CRA/ Advanced configuration.

To define environment variable on Windows OS, une must use set command, ie set PORT=3001 as described here.

Usually I'd recommend using tool such as cross-env, however as PORT may (and by default is recommended to) be set in the .env file I think it's better to move it to .env.dist file:

# See https://create-react-app.dev/docs/advanced-configuration/
# Start development server on port 3001
PORT=3001

This may be considered minor breaking change as dev server will start at default port 3000 unless developer will update one of the local .env* files.

In my opinion this is a minor issue as by default dev server is set up to automatically open new browser with proper URL anyway.

Workaround

node_modules\.bin\react-scripts start
qLb commented 4 years ago

I'm sorry but we supprt UNIX based developer environments. Why don't You try WSL?

piotr-cz commented 4 years ago

Windows is perfectly fine for development for me (especially web apps), for more complex projects I use VMs.

I guess your reason to pick up other port than default (3000) is that you may develop different projects in isolated context (ie. isolated service worker and local storage per origin).

In my opinion this is developers custom variable that should be set in .env.development.local file, not in package script.

However if you really insist on setting port in package script, the cross-os way is to use cross-env:

yarn add --dev cross-env
-   "start": "PORT=3001 react-scripts start",
+   "start": "cross-env PORT=3001 react-scripts start",
qLb commented 4 years ago

I understand and You are absolutely right this should be dealt with using cross-env. Let me call someone from react department so they can fix it right in the downstream. @thecodersio @TomaszMolenda - could we resolve this in the upcoming release (next week)?

arthurkowalsky commented 4 years ago

We will do our best to include this issue in the upcoming release.

arthurkowalsky commented 4 years ago

@piotr-cz Could you please send us the disclaimer ?

piotr-cz commented 4 years ago

yes, I'll do so by the end of this week

piotr-cz commented 4 years ago

disclaimer sent