-
Notifications
You must be signed in to change notification settings - Fork 522
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
ts_project should provide LinkablePackageInfo #1896
Comments
So update on how I got this working: Unfortunately After that, in order for TSC to resolve it (since it's generated file), I added path resolver in root {
"paths": {
"@mylib/*": [
"bazel-out/k8-fastbuild/bin/packages/*",
"bazel-out/darwin-fastbuild/bin/packages/*",
"bazel-out/x64_windows-fastbuild/bin/packages/*",
"bazel-out/k8-dbg/bin/packages/*",
"bazel-out/darwin-dbg/bin/packages/*",
"bazel-out/x64_windows-dbg/bin/packages/*"
],
}
} My custom rule: def _ts_monorepo_subpackage_impl(ctx):
ts_project = ctx.attr.ts_project
ts_project_files = ts_project[DefaultInfo].files.to_list() + ts_project[DeclarationInfo].declarations.to_list()
files = []
is_all_sources = ts_project_files[0].is_source
for src in ts_project_files:
if src.is_source:
dst = ctx.actions.declare_file(src.basename, sibling = src)
copy_file(ctx, src, dst)
files.append(dst)
else:
files.append(src)
files_depset = depset(files)
result = [
DefaultInfo(
files = files_depset,
runfiles = ctx.runfiles(files = ts_project_files),
),
ts_project[DeclarationInfo],
]
path = "/".join([p for p in [ctx.bin_dir.path, ctx.label.workspace_root, ctx.label.package] if p])
result.append(LinkablePackageInfo(
package_name = ctx.attr.package_name,
path = path,
files = files_depset,
))
return result
_ts_monorepo_subpackage = rule(
_ts_monorepo_subpackage_impl,
attrs = {
"package_name": attr.string(
mandatory=True,
doc="Package name in package.json"
),
"ts_project": attr.label(
mandatory=True,
doc="Label for ts_project"
),
},
)
def ts_monorepo_subpackage(
name,
package_name,
**kwargs):
ts_project(
name = "%s-lib" % name,
**kwargs,
)
_ts_monorepo_subpackage(
name = name,
ts_project = ":%s-lib" % name,
package_name=package_name
) At that point |
Before seeing your reply here I was also playing with this to understand the current state, and found the same thing, that #1898 is my little repro. This is pretty easy to fix but we should be careful to fix it in the right way since users will have lots of code that does this and we'll never want to change it.
This makes me think we should fix it in both ways:
Trying to think through the implications of 2) though - if you have a js_library or ts_project with package_name=foo and then want to publish it to npm, you'll wrap with pkg_npm and now we will have two different sources of the foo package_name mapping. That might be a confusing sharp edge, if they conflict? |
Hmm my solution fails when trying to chain subpackages import together, e.g I think (1) makes sense and (2) can optionally have a |
|
It seems like your thought process could be geared toward the library author’s use case; for those that publish to packages to npm. I would like to speak up as one who has migrated to bazel but is not publishing to npm rather only building containers of apps for deployment. I do not think that the package.json should be the primary method of specifying the module name for a ts_project, I think it does deserve its own package_name. Before bazel we were using rush.js (and before that lerna) and therefore each intermediate package had a package.json, but after migrating to bazel we found it freeing to be rid of that constraint: The classic npm package name is restricting! You can only have 1 slash; between scope and name. We found the scope of the project ends up not being all that informative, just a common name space that all our packages shared. I found it much better (albeit different from the node ecosystem) to be able to import a package using the pathname in the repo itself, or a pathname from a common root location. We found this more Intuitive. It was very freeing to also support nested packages, that are reflected in the package name as well. It has led to more intuitive organization and discovery of related packages. All that to say, I really appreciated the flexibility of an arbitrary module_name of ts_library and think something just as flexible should be used for ts_project. |
I'd advocate for either no |
We aren't currently using |
I don't think that's what I'm suggesting. If you need to refer to a |
#1897 includes a way for pkg_npm generate package.json if none is provided. Certainly won't require having one, especially because of the first-party-use-only use case |
@gregmagolan helped me remember a design flaw with However However the fact that a first-party pkg_npm doesn't behave like a third-party node_module_library is disconcerting. We should think about this some more. |
In the mean time are there any recommendations to solve this issue? |
in our sync meeting we decided to go ahead with optionally, we could copy a package.json file across to the bin folder, but it feels like that should be the job of pkg_npm |
yup it does! :) |
Discussion in team meeting: we think we want a single rule to be the hub in a hub-and-spoke model, so that we don't end up sprinkling assets and package_name in a bunch of places. |
This issue has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs in two weeks. Collaborators can add a "cleanup" or "need: discussion" label to keep it open indefinitely. Thanks for your contributions to rules_nodejs! |
We have js_library now |
Going public API with it: #2187 |
Just to be clear, the fix here was to have |
correct, the intermediate rule is the moral equivalent of installing a package. This should only be needed when you want to import it with a "short" package name rather than a fully-qualified repo path or a relative path |
Given the following:
Everything works great if both BUILD files have What am I doing wrong? I understood from Alex's last message that relative paths should work? |
@tomasdev it depends on your To understand it better, try running Add You can trace through from here OTOH if you use |
Currently a ts_project needs a js_library or pkg_npm between it and another ts_project, if the latter wants to import it with a package name rather than a relative import.
We should investigate if that intermediate rule is really necessary.
The text was updated successfully, but these errors were encountered: