Skip to content
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

Support changing working directory in npm_package_bin #1840

Closed
hsyed opened this issue Apr 21, 2020 · 17 comments
Closed

Support changing working directory in npm_package_bin #1840

hsyed opened this issue Apr 21, 2020 · 17 comments

Comments

@hsyed
Copy link

hsyed commented Apr 21, 2020

🚀 feature request

Relevant Rules

Description

I'm trying to build a vue application (details #1831). I have had success but the resulting application is not quite correct yet. npm_package_bin runs from the root of the workspace however the vue cli tool expects be run in the root of vue project to pick up various build config files and directories.

Describe the solution you'd like

To get past the problem we need npm_package_bin to support changing the working directory to the current package.

@alexeagle
Copy link
Collaborator

alexeagle commented Apr 24, 2020

We could make this a built-in feature but it's pretty easy to do in userspace.

load("@bazel_skylib//rules:write_file.bzl", "write_file")

write_file(
    name = "write_chdir_script",
    out = "chdir.js",
    content = ["process.chdir(__dirname)"],
)

vue-cli-service(
    ...
    args = ["--node_options=--require=./$(execpath chdir.js)"],
    data = ["chdir.js"],
)

@alexeagle
Copy link
Collaborator

Note that I discussed this with @gregmagolan and the reason we didn't build it in yet is that lots of bazel path support expects "workspace-absolute" paths, so while that code snippet is working for me in a react-scripts scenario we need to think through bad interactions it might have

@weixiao-huang
Copy link

weixiao-huang commented Apr 25, 2020

this method may hang so much time and due to timeout, I try to get the real exec command as follow

/root/.cache/bazel/_bazel_root/install/d687aca87a7669724cc958527f2423da/process-wrapper --timeout=0 --kill_delay=15 bazel-out/host/bin/external/npm/@vue/cli-service/bin/vue-cli-service.sh build --dest=bazel-out/k8-fastbuild/bin/portal --node_options=--require=./bazel-out/k8-fastbuild/bin/portal/chdir.js --bazel_node_modules_manifest=bazel-out/k8-fastbuild/bin/portal/_build.module_mappings.json --nobazel_patch_module_resolver

I exec it by hand in the workdir and got the error

Error: Cannot find module './bazel-out/host/bin/external/npm/@vue/cli-service/bin/vue-cli-service.sh.runfiles/build_bazel_rules_nodejs/internal/node/node_patches.js'
Require stack:
- internal/preload
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:982:15)
    at Function.Module._load (internal/modules/cjs/loader.js:864:27)
    at Module.require (internal/modules/cjs/loader.js:1044:19)
    at Module._preloadModules (internal/modules/cjs/loader.js:1298:12)
    at loadPreloadModules (internal/bootstrap/pre_execution.js:444:5)
    at prepareMainThreadExecution (internal/bootstrap/pre_execution.js:68:3)
    at internal/main/run_main_module.js:7:1 {
  code: 'MODULE_NOT_FOUND',
  requireStack: [ 'internal/preload' ]
}
internal/modules/cjs/loader.js:985
  throw err;
  ^

So I don't know why, so I use a hack method to bypass this implementation in #1776 (comment)

Does anyone know why this error happened above?

@dubov94
Copy link

dubov94 commented Apr 25, 2020

@weixiao-huang Could you provide a complete minimal (non-)working example? Alex's suggestion looks reasonable to me.

Once again, if you want a module that 'just works' out of the box here it is. It's a launcher for vue-cli-service that takes an additional option (--package-json-path) and sets VUE_CLI_CONTEXT accordingly without even touching the current working directory.

@weixiao-huang
Copy link

weixiao-huang commented Apr 25, 2020

@dubov94 See this: https://github.com/weixiao-huang/bazel-vue-example
If I use bazel build portal:build, it will hang. My bazel version is 1.2.0. And this portal vue project is created by vue create portal and use options below:
image

@dubov94
Copy link

dubov94 commented Apr 25, 2020

@alexeagle Unfortunately $(@D) is relative to execroot/workspace, and vue-cli-service build resolves relative --dest based on package.json location (which is either VUE_CLI_CONTEXT or just process.cwd()), so generated files end up in a wrong place.

We could obviously prepend $(@D) with enough double dots to get to execroot/workspace eventually, but this does not look nice at all and, IMO, requires too much user knowledge about how output directories work.

@dubov94
Copy link

dubov94 commented Apr 25, 2020

@weixiao-huang Thank you, I was able to reproduce the problem, and I don't see an immediate solution. For now you can use vue-cli-launcher instead of vue-cli-service.

@alexeagle Isn't actually --node_options supposed to be a part of tool somehow?

@alexeagle
Copy link
Collaborator

Yeah messing up the output directory is one of the follow-on path issues from changing the working directory that I mentioned earlier, prevents us from making this chdir trick built-in right now.

--node_options shouldn't be part of tool if you want to be able to pass different options to different invocations of the tool, right?

@weixiao-huang I don't think your hang is related to the original issue of setting the working directory. Note that if you want to debug by calling the bazel exec command directly, you must also run it in the same working directory (this is why bazel --subcommands prints the command wrapped in a subshell with a cd)

Let's keep this issue specifically for the working directory, I think there are other issues about vue (would be great to have an example checked in)

@dubov94
Copy link

dubov94 commented Apr 27, 2020

I agree that ideally the tool should be target-independent, but in your suggestion aren't we passing --node_modules= to vue-cli-service rather than Node?

Looks like @weixiao-huang's bazel-vue-example does not hang if we remove --node_options= from args.


Anyway, even if we manage to find a shortcut for this particular case, it's likely to be quite fragile. Perhaps it shouldn't be handled on rules_nodejs level in the first place. We could probably distinguish a class of instruments that expect both inputs and outputs to be located under a single directory containing package.json, and implement dedicated tooling for it — for example, a wrapper creating a temporary directory of the required structure and then copying the results back where they actually are expected.

@hsyed
Copy link
Author

hsyed commented May 5, 2020

Apologies complete node and JS compilation noob here.Tried the approach above in a few forms, in the form below it hangs:

load("@build_bazel_rules_nodejs//:index.bzl", "npm_package_bin")
load("@bazel_skylib//rules:write_file.bzl", "write_file")

write_file(
    name = "write_chdir_script",
    out = "chdir.js",
    content = ["process.chdir(__dirname)"],
)

npm_package_bin(
    name = "app",
    tool = "@npm//@vue/cli-service/bin:vue-cli-service",
    args = [
        "build",
        "--dest", "$(@D)",
        "--node_options=--require=./$(execpath chdir.js)"
    ],
    data = [
        "@npm//:node_modules",
        "chdir.js"
    ] + glob(["src/**"]),
    output_dir = 1,
    visibility = ["//visibility:private"],
)

If I adapt the above to provide the entry point main.js file to the vue-cli-service it cannot resolve it.

I tried to replicate the workaround by executing in the root of our monorepo and get the following:

➜  esqimo-platform git:(sandbox/bazel-js) ✗ ./js/node_modules/.bin/vue-cli-service build js/src/main.js --node_options=--require=js/chdir.js

⠇  Building for production...

 ERROR  Failed to compile with 13 errors                                                                                                                                                                                                                                                                                                                                                                                                                                               10:32:18 AM

Failed to resolve loader: cache-loader
You may need to install it.
Failed to resolve loader: cache-loader
You may need to install it.
Failed to resolve loader: cache-loader
You may need to install it.
Failed to resolve loader: cache-loader
You may need to install it.
Failed to resolve loader: cache-loader
You may need to install it.
Failed to resolve loader: cache-loader
You may need to install it.
Failed to resolve loader: cache-loader
You may need to install it.
Failed to resolve loader: cache-loader
You may need to install it.
Failed to resolve loader: cache-loader
You may need to install it.
Failed to resolve loader: cache-loader
You may need to install it.
Failed to resolve loader: cache-loader
You may need to install it.
Failed to resolve loader: cache-loader
You may need to install it.
Failed to resolve loader: css-loader
You may need to install it.
 ERROR  Build failed with errors.

@hsyed
Copy link
Author

hsyed commented May 15, 2020

@alexeagle any thoughts on the best way forward ?

I think vue-cli-service should support a parameter telling it where the project is located. I could open an issue for this ?

@alexeagle
Copy link
Collaborator

Hey, yeah I agree that's a good FR for vue-cli-service, I'd recommend phrasing the issue in terms of "monorepo" rather than mention Bazel since the latter has a narrow audience right now and won't get their attention.

I think for us to dig into the vue-specific parts of this issue, the best way is to add examples/vue to this repo so we can have some first-party-maintained bits that we understand and run tests against. Then ppl with vue-specific issues can hand us a repro just in terms of changes to that example which add their use case.

Anyone on the thread have time to contribute that example?

@dubov94
Copy link

dubov94 commented May 21, 2020

I'm pretty sure vuejs/vue-cli#3150 and its references address this, but there hasn't been much activity going on recently.

Do I understand correctly that you are suggesting to add the non-working example to examples/vue?

@Smokrow
Copy link
Contributor

Smokrow commented May 24, 2020

Would the example be a multiproject repo or just for VueJS?

@alexeagle
Copy link
Collaborator

Ideally we add a working VueJS example. There's so much discussion above I'm not sure if there is any working vue setup today?
Even if it's not working, at least we would have a red PR containing a vue app that I can iterate on to fix it.

@Smokrow
Copy link
Contributor

Smokrow commented May 26, 2020

I am currently setting one up using the vue-cli-service tool. Had a few troubles running the build command though. I will see what I can do. If you want I could just add a standard default Vue application and put some build files in it once I got it working.

@alexeagle
Copy link
Collaborator

#1915 has the vue example.
vuejs/vue-cli#3152 has a possible solution for this that doesn't require chdir.
I'll document/test the chdir userspace workaround as a way to close this issue.

alexeagle pushed a commit to alexeagle/rules_nodejs that referenced this issue Jul 5, 2020
alexeagle pushed a commit to alexeagle/rules_nodejs that referenced this issue Jul 5, 2020
alexeagle pushed a commit to alexeagle/rules_nodejs that referenced this issue Jul 5, 2020
alexeagle pushed a commit to alexeagle/rules_nodejs that referenced this issue Jul 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants