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

Throw an exception if NODE_ENV is not set when running webpack #583

Closed
wants to merge 2 commits into from
Closed

Throw an exception if NODE_ENV is not set when running webpack #583

wants to merge 2 commits into from

Conversation

soundasleep
Copy link
Contributor

On a brand new install, if NODE_ENV is not already set, then
running bin/webpack will fail with an obscure error message:

  path: resolve('public', settings.public_output_path),
                                  ^

TypeError: Cannot read property 'public_output_path' of undefined
    at Object.<anonymous> (.../config/webpack/configuration.js:26:35)

Since someone will never be able to run webpack without NODE_ENV set,
it makes sense to give the developer a message to help them debug.

On a brand new install, if NODE_ENV is not already set, then 
running `bin/webpack` will fail with an obscure error message:

  path: resolve('public', settings.public_output_path),
                                  ^

TypeError: Cannot read property 'public_output_path' of undefined
    at Object.<anonymous> (.../config/webpack/configuration.js:26:35)

Since someone will never be able to run webpack without NODE_ENV set,
it makes sense to give the developer a message to help them debug.
Throw an exception if NODE_ENV is not set when running webpack
@gauravtiwari
Copy link
Member

@soundasleep Thanks for this but we already set this inside binstub - https://github.com/rails/webpacker/blob/master/lib/install/bin/webpack.tt#L10

@soundasleep
Copy link
Contributor Author

I realised that later... I was having issues with yarn not playing nicely on Windows (it was trying to execute using the wrong path separators), so was having to run webpack manually, which caused this. Happy to close :)

@gauravtiwari
Copy link
Member

@soundasleep Could you please help me debug that? It seems that many people are having same problem on windows but I don't have access to a windows machine.

@soundasleep
Copy link
Contributor Author

soundasleep commented Jul 22, 2017

Absolutely! I'd be happy to help, what would you like me to do? Here's some of my history:

You can't run something with a forward slash in Windows:

C:\path>bin/webpack
'bin' is not recognized as an internal or external command,
operable program or batch file.

So, I used backslash. But because it isn't a .bat or .cmd file, Windows doesn't read the first line of the script as the interpreter to use:

C:\path>bin\webpack
'bin\webpack' is not recognized as an internal or external command,
operable program or batch file.

So I then started passing along the script to Ruby. But then the path separator thing started occurring:

C:\path>ruby bin\webpack
yarn run v0.27.5
warning package.json: No license field
$ "C:\path\node_modules\.bin\webpack" "--config" "C:/path/config/webpack/development.js"
The filename, directory name, or volume label syntax is incorrect.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

What I've done, in the meantime as a workaround, is the following in my bin/webpack:

# ... etc

newenv  = { "NODE_PATH" => NODE_MODULES_PATH.shellescape }
if Gem.win_platform?
  cmdline = ["#{NODE_MODULES_PATH}/.bin/webpack", "--config", WEBPACK_CONFIG] + ARGV
else
  cmdline = ["yarn", "run", "webpack", "--", "--config", WEBPACK_CONFIG] + ARGV
end

Dir.chdir(APP_PATH) do
  exec newenv, *cmdline
end

And now if I go ruby bin/webpack, it works fine.

It might be an issue with Yarn but I'm not confident.

@gauravtiwari
Copy link
Member

Thank you so much. This is great 👍 🍰

Perhaps, you wanna make a PR with your solution until yarn is fixed. I saw many issues raised with same problem. Does the solution works with foreman too?

@gauravtiwari
Copy link
Member

I guess we will also need to document the README to add ruby prefix when running these binstubs on windows

soundasleep added a commit to soundasleep/webpacker that referenced this pull request Jul 22, 2017
- Bypass `yarn run` when running on Windows; instead, we can go directly
to `node_modules/.bin/`
- Create batch scripts so users don't need to type in `ruby bin/webpack`
manually

See discussion on rails#583
@soundasleep
Copy link
Contributor Author

How's this? ^_^ #584

@gauravtiwari
Copy link
Member

closing in favour of #584

gauravtiwari pushed a commit that referenced this pull request Jul 24, 2017
* Workarounds for Yarn not playing nicely on Windows

- Bypass `yarn run` when running on Windows; instead, we can go directly
to `node_modules/.bin/`
- Create batch scripts so users don't need to type in `ruby bin/webpack`
manually

See discussion on #583

* Update Changelog and README for Webpacker on Windows

Do not generate batch files when installing
sensiblegame added a commit to sensiblegame/webpack that referenced this pull request Oct 23, 2017
* Workarounds for Yarn not playing nicely on Windows

- Bypass `yarn run` when running on Windows; instead, we can go directly
to `node_modules/.bin/`
- Create batch scripts so users don't need to type in `ruby bin/webpack`
manually

See discussion on rails/webpacker#583

* Update Changelog and README for Webpacker on Windows

Do not generate batch files when installing
KingTiger001 added a commit to KingTiger001/Rails-web-pack-project that referenced this pull request Jan 15, 2023
* Workarounds for Yarn not playing nicely on Windows

- Bypass `yarn run` when running on Windows; instead, we can go directly
to `node_modules/.bin/`
- Create batch scripts so users don't need to type in `ruby bin/webpack`
manually

See discussion on rails/webpacker#583

* Update Changelog and README for Webpacker on Windows

Do not generate batch files when installing
smartech7 pushed a commit to smartech7/ruby-webpacker that referenced this pull request Aug 4, 2023
* Workarounds for Yarn not playing nicely on Windows

- Bypass `yarn run` when running on Windows; instead, we can go directly
to `node_modules/.bin/`
- Create batch scripts so users don't need to type in `ruby bin/webpack`
manually

See discussion on rails/webpacker#583

* Update Changelog and README for Webpacker on Windows

Do not generate batch files when installing
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants