Breaking change since ESBuild >0.16 chore commit #55
Conversation
|
Its actually due to this #32 |
| if (IS_WATCH_MODE) { | ||
| await context.watch(); | ||
| } else { | ||
| await context.rebuild(); |
There was a problem hiding this comment.
I'm not entirely sure if esbuild logs errors internally, even though this does happen when using watch mode. Maybe it would be safe to still add a check like we had before.
| console.error(`[ESBuild] Bundle failed with ${errors.length} errors`); | ||
| process.exit(1); | ||
| if (IS_WATCH_MODE) { | ||
| await context.watch(); |
There was a problem hiding this comment.
Redundant await
| await context.watch(); | |
| context.watch(); |
There was a problem hiding this comment.
Not sure if we want to include a package-lock.json, I'll let @itschip decide that I guess
|
Seems like you've installed some packages with npm instead of yarn. Please remove the package-lock file and install again with yarn to update the yarn.lock file :P |
This PR is pretty old, so if you want I'll make a new one with the requested changes. I doubt the author is still active 😄 |
|
I'm alive and kicking. I just didn't reply to the original comments - must have lost them in the email spam.
I've left a comment explaining that this PR can be ignored until ESBuild version is increased by FiveM. Since the node version is unsupported by peer dependencies etc. I can make changes if needed but yeah as per above. Edit: Instead the version of ESBuild should be rolled back since its a breaking change and FiveM doesn't support it. |
We can roll back the version of ESBuild, but I'm quite sure that NPWD is not meant to be built on the server in the first place. Thus, I think it would be fine, but I also agree that the impact of downgrading is pretty much nothing, so I'm going to agree. |
|
While NPWD may not be impacted - this repo is meant to act as a boilerplate (as far as I understand) which in its current form it isn't, therefore it shouldn't be public and/or exist. Most beginners/average users will be met with frustration and errors, as the issue when using this standalone is not obvious since its the FiveM Node version at fault. I'm also going to say on a whim that NPWD probably has its own esbuild stuff in its repo and is not using this either directly or indirectly in any shape or form, hence rolling back on the esbuild will not impact NPWD. Best approach (imo) would be to roll it back close this PR and mention that this ESBuild version is a strict dependency linking to this PR and the discussion. I hope that clarifies why I left the discussion open :) |
|
I totally forgot that this was about the boilerplate, my apologies 😂 Your solution is good, I agree. |
Pull Request Description
ESBuild build got bumped in this commit db0059b leading to a breaking change.
ESBuild 0.17 introduced a breaking (backwards-incompatible) change to
.build()- SEE HEREChore commit was blindly merged breaking the
watchandbuildcommands.This basically makes it work with latest esbuild and simplifies the
build-bundle.js.(Edit side note: I used npm hence the lock file. Welcome to delete or ignore it. The main important part is the
build-bundle.js)Pull Request Checklist:
resourcein-game after the relevant change?