Merged
Conversation
Contributor
Author
|
1. use finalAttrs param instead of rec attrs using a "fix-point" style derivation that takes a self arg (aka finalAttrs) makes it easier to override things for consumers than when using a `rec` attr set, which is just syntactic sugar and doesn't propagate overriding. this is especially useful since the the build's ldflags and the web sub-derivation definition both used the `src` and `version` attributes. 2. refact esbuild override the derivation forces esbuild to be the same version as declared in its web/pnpm-lock.yaml file, but it did it by overriding the definition of `buildGoModule` used by the esbuild drv. however, it seems like this was unecessary, since just doing .overrideAttrs also worked fine for changing the esbuild version.
872d181 to
c2b51e4
Compare
Contributor
Author
|
Forgot to run |
eriedaberrie
approved these changes
Feb 2, 2026
Member
eriedaberrie
left a comment
There was a problem hiding this comment.
This derivation was created before #390220, hence the rec and the incredibly hacky esbuild version overriding; refactoring it to be more modern/extensible is definitely a welcome change. Outside of the scope of this PR: from what I can tell (running a quick grep for esbuild.override) there are still a decent number of packages overriding esbuild in this manner, I guess can also be updated at some point.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Updates
syncyomifrom 1.1.2 to 1.1.4.Changelog:
I also did some small refactoring to the derivation so that it's easier to override (it used
recattrsets, it now usesfinalAttrs: ...) and more conventional (the way theesbuildwas pinned to an older version was a little confusing/hackish). Right now those changes are in a separate commit from the update, but I can squash them into just one if that's better (I'm not sure if that counts as a "logical unit" based on the contributing guidelines).Things done
passthru.tests.nixpkgs-reviewon this PR. See nixpkgs-review usage../result/bin/.