refactor: group fp package linking alongside third-party#2427
refactor: group fp package linking alongside third-party#2427jbedard merged 1 commit intoaspect-build:mainfrom
Conversation
|
| link_68("{}/uvu".format(name), False, name, "uvu") | ||
| link_targets.append(":{}/uvu".format(name)) | ||
| _fp_link_0(name) | ||
| link_targets.append(":{}/@scoped/c".format(name)) |
There was a problem hiding this comment.
ideally we'd generate a bit tighter code here and didn't keep checking for @scoped not in scope_targets
There was a problem hiding this comment.
That code has simply moved and is not new. But now it is alongside the third-party code (which does the exact same thing) so I hope we can cleanup both together now, coming soon...
| # Generated npm_link_package_store for linking of first-party "@scoped/c" package | ||
| # buildifier: disable=function-docstring | ||
| def _fp_link_0(name): | ||
| _npm_link_package_store( |
There was a problem hiding this comment.
probably want to package these up into a macro in the rules_js repo and invoke that with the extra args it needs, to generate less code?
There was a problem hiding this comment.
You mean similar to what _LINK_JS_PACKAGE_LINK_IMPORTED_STORE_TMPL in npm_import.bzl is doing?
| for link_package in all_fp_link_packages.keys(): | ||
| if link_package not in links_pkg_bzl: | ||
| links_pkg_bzl[link_package] = [] | ||
| links_pkg_bzl[link_package].append(""" _fp_link_{i}(name)""".format(i = i)) |
There was a problem hiding this comment.
can we arrange to do this first so link_targets is empty before this, and then the below will essentially reduce to scope_targets["{package_scope}"] = list(link_targets)?
There was a problem hiding this comment.
Hmm... now that this PR is moving all first+third-party into a single if bazel_package == "x" block I think you're right... it can probably change to a single assignment (per scope) instead of all this appending.
That will probably be in a followup...
dzbarsky
left a comment
There was a problem hiding this comment.
feel free to do the codegen cleanups in a followup
Generate factory methods instead of inline code within
npm_link_all_packagesto better align first-party package linking with third-party.Changes are visible to end-users: no
Test plan