Skip to content

Commit

Permalink
chore: change all internal virtual store naming to package store (#1616)
Browse files Browse the repository at this point in the history
  • Loading branch information
gregmagolan committed May 20, 2024
1 parent 09e4546 commit df7608e
Show file tree
Hide file tree
Showing 31 changed files with 159 additions and 159 deletions.
2 changes: 1 addition & 1 deletion BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ load("//npm:defs.bzl", "npm_link_package")
load("//js:defs.bzl", "js_library")

# Link all packages from the /WORKSPACE npm_translate_lock(name = "npm") and also packages from
# manual /WORKSPACE npm_import rules to bazel-bin/node_modules as well as the virtual store
# manual /WORKSPACE npm_import rules to bazel-bin/node_modules as well as the package store
# bazel-bin/node_modules/.aspect_rules_js since /pnpm-lock.yaml is the root of the pnpm workspace
npm_link_all_packages(
name = "node_modules",
Expand Down
2 changes: 1 addition & 1 deletion MODULE.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ npm.npm_import(
bins = {"acorn": "./bin/acorn"},
integrity = "sha512-ULr0LDaEqQrMFGyQ3bhJkLsbtrQ8QibAseGZeaSUiT/6zb9IvIkomWHJIvgvwad+hinRAgsI51JcWk2yvwyL+w==",
package = "acorn",
# Root package where to link the virtual store
# Root package where to link the package store
root_package = "",
version = "8.4.0",
)
Expand Down
2 changes: 1 addition & 1 deletion WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ npm_import(
bins = {"acorn": "./bin/acorn"},
integrity = "sha512-ULr0LDaEqQrMFGyQ3bhJkLsbtrQ8QibAseGZeaSUiT/6zb9IvIkomWHJIvgvwad+hinRAgsI51JcWk2yvwyL+w==",
package = "acorn",
# Root package where to link the virtual store
# Root package where to link the package store
root_package = "",
version = "8.4.0",
)
Expand Down
6 changes: 3 additions & 3 deletions docs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ Installation is included in the `WORKSPACE` snippet you pasted from the Installa
### Fetch third-party packages from npm

rules_js accesses npm packages using [pnpm].
pnpm's "virtual store" of packages aligns with Bazel's "external repositories",
pnpm's ["symlinked node_modules structure"](https://pnpm.io/symlinked-node-modules-structure) of packages aligns with Bazel's "external repositories",
and the pnpm "linker" which creates the `node_modules` tree has semantics we can reproduce with Bazel actions.

If your code works with pnpm, then you should expect it works under Bazel as well.
Expand Down Expand Up @@ -132,9 +132,9 @@ You can see this working by running `bazel build ...`, then look in the `bazel-b
You'll see something like this:

```bash
# the virtual store
# the package store
bazel-bin/node_modules/.aspect_rules_js
# symlink into the virtual store
# symlink into the package store
bazel-bin/node_modules/some_pkg
# If you used pnpm workspaces:
bazel-bin/packages/some_pkg/node_modules/some_dep
Expand Down
4 changes: 2 additions & 2 deletions docs/js_run_devserver.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion docs/npm_import.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 4 additions & 4 deletions docs/npm_link_package.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion docs/troubleshooting.md
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ The documentation says the value provided to each element in the map is:
> a list of Bazel packages in which to hoist the package to the top-level of the node_modules tree
To make plugins work, you should have the Bazel package containing the pnpm workspace root (the folder containing `pnpm-lock.yaml`) in this list.
This ensures that the tool in the pnpm virtual store `node_modules/.aspect_rules_js` will be able to locate the plugins.
This ensures that the tool in the package store (`node_modules/.aspect_rules_js`) will be able to locate the plugins.
If your lockfile is in the root of the Bazel workspace, this value should be an empty string: `""`.
If the lockfile is in `some/subpkg/pnpm-lock.yaml` then `"some/subpkg"` should appear in the list.

Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 4 additions & 4 deletions js/private/js_run_devserver.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,12 @@ def _js_run_devserver_impl(ctx):
)]

# The .to_list() calls here are intentional and cannot be avoided; they should be small sets of
# files as they only include direct npm links (node_modules/foo) and the virtual store tree
# files as they only include direct npm links (node_modules/foo) and the package store tree
# artifacts those symlinks point to (node_modules/.aspect_rules_js/foo@1.2.3/node_modules/foo)
data_files = []
for f in depset(transitive = transitive_runfiles + [dep.files for dep in ctx.attr.data]).to_list():
if "/.aspect_rules_js/" in f.path:
# Special handling for virtual store deps; we only include 1st party deps since copying
# Special handling for package store deps; we only include 1st party deps since copying
# all 3rd party node_modules over is expensive for typical graphs
path_segments = f.path.split("/")
package_name_segment = path_segments.index(".aspect_rules_js") + 1
Expand Down Expand Up @@ -196,8 +196,8 @@ def js_run_devserver(
The custom sandbox is populated with the default outputs of all targets in `data`
as well as transitive sources & npm links.
As an optimization, virtual store files are explicitly excluded from the sandbox since the npm
links will point to the virtual store in the execroot and Node.js will follow those links as it
As an optimization, package store files are explicitly excluded from the sandbox since the npm
links will point to the package store in the execroot and Node.js will follow those links as it
does within the execroot. As a result, rules_js npm package link targets such as
`//:node_modules/next` are handled efficiently. Since these targets are symlinks in the output
tree, they are recreated as symlinks in the custom sandbox and do not incur a full copy of the
Expand Down
8 changes: 4 additions & 4 deletions js/private/js_run_devserver.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ export function isNodeModulePath(p) {
return false
}

// Determines if a file path is a 1p dep in the virtual store.
// Determines if a file path is a 1p dep in the package store.
// See js/private/test/js_run_devserver/js_run_devserver.spec.mjs for examples.
export function is1pVirtualStoreDep(p) {
// unscoped1p: https://regex101.com/r/hBR08J/1
Expand Down Expand Up @@ -258,12 +258,12 @@ async function sync(files, sandbox, writePerm) {

if (virtualStore1pFiles.length > 0 && process.env.JS_BINARY__LOG_DEBUG) {
console.error(
`Syncing ${virtualStore1pFiles.length} first party virtual store dep(s)`
`Syncing ${virtualStore1pFiles.length} first party package store dep(s)`
)
}

// Sync first-party virtual store files first since correctly syncing direct 1p node_modules
// symlinks depends on checking if the virtual store synced files exist.
// Sync first-party package store files first since correctly syncing direct 1p node_modules
// symlinks depends on checking if the package store synced files exist.
let totalSynced = (
await Promise.all(
virtualStore1pFiles.map(async (file) => {
Expand Down
4 changes: 2 additions & 2 deletions js/private/test/js_run_devserver/js_run_devserver.spec.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ const is1pVirtualStoreDep_true = [
]
for (const p of is1pVirtualStoreDep_true) {
if (!is1pVirtualStoreDep(p)) {
console.error(`ERROR: expected ${p} to be a 1p virtual store dep`)
console.error(`ERROR: expected ${p} to be a 1p package store dep`)
process.exit(1)
}
}
Expand All @@ -48,7 +48,7 @@ const is1pVirtualStoreDep_false = [
]
for (const p of is1pVirtualStoreDep_false) {
if (is1pVirtualStoreDep(p)) {
console.error(`ERROR: expected ${p} to not be a 1p virtual store dep`)
console.error(`ERROR: expected ${p} to not be a 1p package store dep`)
process.exit(1)
}
}
Expand Down
18 changes: 9 additions & 9 deletions js/private/test/node-patches/realpath.js
Original file line number Diff line number Diff line change
Expand Up @@ -513,11 +513,11 @@ describe('testing realpath', async () => {
{
sandbox: {
node_modules: {},
virtual_store: { pkg: {} },
package_store: { pkg: {} },
},
execroot: {
node_modules: {},
virtual_store: {
package_store: {
pkg: {
file: 'contents',
},
Expand All @@ -527,9 +527,9 @@ describe('testing realpath', async () => {
async (fixturesDir) => {
fixturesDir = fs.realpathSync(fixturesDir)

// create symlink from execroot/node_modules/pkg to execroot/virtual_store/pkg
// create symlink from execroot/node_modules/pkg to execroot/package_store/pkg
fs.symlinkSync(
path.join(fixturesDir, 'execroot', 'virtual_store', 'pkg'),
path.join(fixturesDir, 'execroot', 'package_store', 'pkg'),
path.join(fixturesDir, 'execroot', 'node_modules', 'pkg')
)

Expand All @@ -538,20 +538,20 @@ describe('testing realpath', async () => {
path.join(
fixturesDir,
'execroot',
'virtual_store',
'package_store',
'pkg',
'file'
),
path.join(
fixturesDir,
'sandbox',
'virtual_store',
'package_store',
'pkg',
'file'
)
)
fs.symlinkSync(
path.join(fixturesDir, 'sandbox', 'virtual_store', 'pkg'),
path.join(fixturesDir, 'sandbox', 'package_store', 'pkg'),
path.join(fixturesDir, 'sandbox', 'node_modules', 'pkg')
)

Expand All @@ -569,7 +569,7 @@ describe('testing realpath', async () => {
const filePath = path.join(
fixturesDir,
'sandbox',
'virtual_store',
'package_store',
'pkg',
'file'
)
Expand Down Expand Up @@ -613,7 +613,7 @@ describe('testing realpath', async () => {
const filePath2 = path.join(
fixturesDir,
'sandbox',
'virtual_store',
'package_store',
'pkg'
)

Expand Down
Loading

0 comments on commit df7608e

Please sign in to comment.