godotengine / godot-csharp-vscode

Debugger and utilities for working with Godot C# projects in VSCode
https://marketplace.visualstudio.com/items?itemName=neikeq.godot-csharp-vscode
MIT License
145 stars 27 forks source link

Fix Webpack Config for Newer NodeJS Versions #41

Closed manuth closed 2 years ago

manuth commented 2 years ago

The current Webpack settings don't work well with the current LTS version (v17) as seen in this issue: https://github.com/webpack/webpack/issues/14532

Changes made in this PR will update the webpack configuration to work around this issue by adding a hashFunction to the output-setting.

raulsntos commented 2 years ago

The current NodeJS LTS is 16.14.2, not 17 (see https://nodejs.org/en/about/releases/), but I'm not opposed to supporting a newer version than the LTS.

I tried building with 17.4.0 and it didn't seem like the config.output.hashFunction: 'xxhash64' was necessary. Can you explain what this does and why it's needed? Since the issue you mentioned is closed it seems this may have been fixed and changing the hashFunction as a workaround may no longer be needed in Webpack 5.70.0.

I also saw that you remove the make package in the docker container to build for open-vsx^1. We should consider removing it, it seems to be problematic in Linux although I believe we might need it for Windows.

manuth commented 2 years ago

Thanks a lot for having a look at it 😄 You're right, adding a hashFunction to the settings doesn't seem to be necessary anymore for webpack 5.70.0. I'll remove this right away.

When having make installed as a dependency, I get this error both on windows and inside the node docker container:

> godot-csharp-vscode@0.2.1 compile
> make build

make ℹ info Invoking build target
node:internal/process/promises:279
            triggerUncaughtException(err, true /* fromPromise */);
            ^

[UnhandledPromiseRejection: This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). The promise rejected with the reason "undefined".] {
  code: 'ERR_UNHANDLED_REJECTION'
}

If you agree, I'll remove the make package.

raulsntos commented 2 years ago

Let me defer to @neikeq on that, since he's the one who added it and probably has more context on why/how we use it.

neikeq commented 2 years ago

I honestly don't remember what the make package was for, but it could always be re-added if removing it causes a regression.

neikeq commented 2 years ago

Thanks!