Hey @worried-painter-31382, great work on the

npm_distribution

PR! I’m testing it out now and

package_json

is working great with

docker_image

. A few suggestions/things I noticed: 1.

package_json

outputs have a different root than what we would expect. They are skipping a directory, in my case the folder structure is

node/apps/app1

and when using it as a dependency for other targets they can only be seen as

apps/app1

. Is that expected? 2. It would be great if we could use

extra_env_vars

to set up environment variables for

node_build_script

. Many projects need compile time env vars to create a bundle. 3. What do you think about providing access to the generated “installation” target? That way other targets could use the installed

node_modules

without having to re-install them

1. I'm not following. Where are you observing this? Via

node_build_script

? I am fairly sure this more to do with how pants is configured w.r.t build root than the js backend, specifically 2. I'll make an issue for it! 3. Where are you seeing re-installations? I would expect the pants & package manager caches to handle this. Additonally for 3. I'm fairly sure it is a good decision on pants end to take full ownership and not expose node_modules to end users in ~all goals. That said, I know there are use cases for bundling applications including node_modules, prime example is FaaS. I'm open to suggestions here on paths forward, I dont think there's a "pex" equivalent we could piggypack off of. One obvious but not very nice solution is to copy node_modules in a

node_build_script

.

I’m not following. Where are you observing this? Via

node_build_script

? I am fairly sure this more to do with how pants is configured w.r.t build root than the js

Sorry, this one is hard to explain by text. Let me try again with examples. My project has the following structure:

root
|----> node
|----|---- BUILD
|----|---- app1
|----|----|--- BUILD
|----|----|--- Dockerfile
|----|----L___ index.ts
|----L---- app2

App1/BUILD has a

package_json

target and a

docker_image

target.

package_json

has a

node_build_script

with

output_directory='dist'

If I run

pants package //node/app1:docker --keep-sandboxes=always

my sandbox ends up with the following structure:

root
|----> node
|----|---- app1
|----|-----L____ Dockerfile
|----| app1
|----|--L____ dist/

When I think the most logical structure to end up would be:

root
|----> node
|----|---- app1
|----|----|---- dist/
|----|----L____ Dockerfile

Does that make a little more sense now?

Ah, I see. Yea I think my efforts are better aimed at typescript support rather than exposing the node_modules folder in an interim, and also treat FaaS as a seperate target like python backend does. As for the dist thing, Im not sure exactly how build output paths are computed, but Im fairly sure if you run the package goal or use the docker backend the locations are the same https://www.pantsbuild.org/v2.16/docs/docker with a dotted path. If that isnt the case I think youve found a bug

I think I might have found a bug then bc when I’m using only

adhoc_tool

with an output folder the paths are computed just like the docker build package is (meaning the output folder is in the same path as dockerfile is, for example)

adhoc tool does not produce a package artefact though, it produces

files

Yeah, what surprised me is that it’s in

/apps/app1/dist

instead of

/node/apps/app1/dist

. It just seems weird that it skipped a directory but kept the rest of the folder structure, you know?

What are your source roots?

files(
    name = "project-files",
    sources = [
        "**/*",
        "!**/node_modules/**",
        "!dist/**",
        "!.next/**",
        "!coverage/**",
        "!Dockerfile",
        "!BUILD",
        ".DS_Store",
    ],
)

package_json(
    name = "package-json",
    dependencies = [
        ":project-files",
        "//node:workspace-config",
        "//node:base-config",
    ],
    scripts = [
        node_build_script(
            entry_point = "build",
            output_directories = ["dist/.next"],
        ),
    ],
)

This is how I’m using it

Looks great! Ye, that'd be awesome.