aspect-build / rules_esbuild

Bazel rules for https://esbuild.github.io/ JS bundler
https://docs.aspect.build/rules/aspect_rules_esbuild
Apache License 2.0
27 stars 27 forks source link

fix: don't preserve symlinks so we can resolve npm deps in the symlinked node_modules tree #57

Closed gregmagolan closed 2 years ago

gregmagolan commented 2 years ago

Don't preserve symlinks since doing so breaks node_modules resolution in the pnpm-style symlinked node_modules structure. See https://pnpm.io/symlinked-node-modules-structure.

NB: esbuild will currently leave the sandbox and end up in the output tree until symlink guards are created to prevent this. See https://github.com/aspect-build/rules_esbuild/pull/32.

matthewjh commented 2 years ago

@gregmagolan can you also copy the srcs to a directory and then run esbuild on the files in the directory? When using rules_esbuild, plus preserveSymlinks = False, with rules_ts, esbuild will prefer the .ts files copied into the bindir by rules_ts over .js files intended as inputs, after it follows the symlinks into the bindir.

I had weird bugs from this as esbuild then performs its own typescript transpilation, potentially under a different config depending on which tsconfig, if any, it picks up. My solution was to copy the esbuild srcs to a dir, but I think most people would need this. Of course, this would be removed once the guards are in place.

gregmagolan commented 2 years ago

@gregmagolan can you also copy the srcs to a directory and then run esbuild on the files in the directory? When using rules_esbuild, plus preserveSymlinks = False, with rules_ts, esbuild will prefer the .ts files copied into the bindir by rules_ts over .js files intended as inputs, after it follows the symlinks into the bindir.

I had weird bugs from this as esbuild then performs its own typescript transpilation, potentially under a different config depending on which tsconfig, if any, it picks up. My solution was to copy the esbuild srcs to a dir, but I think most people would need this. Of course, this would be removed once the guards are in place.

Right. I recall @mattem mentioning something like this in the past. Seems like something we could add in the interim until the guards are available. Thanks for the suggestion.

mattem commented 2 years ago

Yeah I've been setting this when I've been using rules_esbuild

config = {
        "resolveExtensions": [".js"],
    },
matthewjh commented 2 years ago

Thanks @gregmagolan @mattem