Implement support for RFC 8, transitive NPM dependencies #606
Comments
|
Hey @alexcrichton, Let me know what you think about my PR #732 |
|
The restriction in the RFC which restricts package.json to only one key |
abernix
added a commit
to apollographql/federation
that referenced
this issue
Sep 23, 2020
…r-wasm` This allows us to compile and depend upon the local version of the WASM query planner package and get the tarballs that are packed with the right version. Additionally, this allows Lerna to handle the version bumping and publishing for `@apollo/query-planner-wasm`, since it is uniquely positioned to do that best in a monorepo orchestration that involves multiple npm packages with relative `file:` dependencies on each other and coupled with our existing needs for Lerna and publishing workflows we have built. Technically speaking, this relieves the `Cargo.toml` from its responsibility of managing the `version` and shifts that responsibility to `lerna` and other npm-specific tooling we have connected to that. In fact, this change completely eliminates the need for the `wasm-pack` generated `package.json` that is derived from the Cargo.toml (and rendered into the "out-dir", previously `pkg/`). While I would believe that there are some advanced `wasm-pack` cases where this above change might be undesirable, for now, the constraint incurred by that transposition is unblocking in other ways. For example, rather than necessitating parallel metadata support in the `Cargo.toml` for every `package.json` property (think about npm specific properties like "labels", "bugs", "private"), we can simply change our `package.json` as we see fit. I am claiming that this side-stepping of version control defined within `Cargo.toml` is acceptable since we do not have any intention of publishing the `query-planner-wasm` package to crates.io on its own. Instead, we will continue to publish the `wasm-pack`'d-from-this-crate `@apollo/query-planner-wasm` package directly to npm's registry. To reflect that desire, I've marked the `query-planner-wasm` package as "private" using the `private = true` property in its `Cargo.toml`. In theory, this change could de-stabilize the work that `wasm-pack` does and that's worth calling out. For example, by not allowing it to generate the `files` property (which indicates which artifacts are emitted into the `npm pack`'d bundle), we might be not allowing it to add other (Future? Unexpected?) important emitted files. I've instead put those `files` directly into the new `package.json` source of truth, along with other appropriate properties, like `types` and `main`. The largest risk here - based on our intended use of the package - seems to be relevant only if we were to rename the host Crate. To demitigate that risk, I've explicitly wired up the `package.json` with `--out-dir` and `--out-file` flags to ensure that it always emits `index`-prefixed files. Furthermore, they are now emitted into the `dist` directory, to be parallel with all our other npm package patterns which do the same. I will note that, `wasm-pack` may address some of the work-arounds here in the future, but best I can tell, they will be roughly compatible with what we're doing here. I am basing this outstanding issues, PRs, RFCs and documentation on the `wasm-pack` project, referenced below. Ref: https://rustwasm.github.io/rfcs/008-npm-dependencies.html#unresolved-questions (See last section) Ref: https://rustwasm.github.io/docs/wasm-pack/commands/build.html#extra-options (See footnote) Ref: rustwasm/wasm-pack#606 Ref: rustwasm/wasm-pack#840
abernix
added a commit
to apollographql/federation
that referenced
this issue
Sep 24, 2020
…r-wasm` (#181) * Setup CircleCI/Lerna/npm to compile and publish `@apollo/query-planner-wasm` This allows us to compile and depend upon the local version of the WASM query planner package and get the tarballs that are packed with the right version. Additionally, this allows Lerna to handle the version bumping and publishing for `@apollo/query-planner-wasm`, since it is uniquely positioned to do that best in a monorepo orchestration that involves multiple npm packages with relative `file:` dependencies on each other and coupled with our existing needs for Lerna and publishing workflows we have built. Technically speaking, this relieves the `Cargo.toml` from its responsibility of managing the `version` and shifts that responsibility to `lerna` and other npm-specific tooling we have connected to that. In fact, this change completely eliminates the need for the `wasm-pack` generated `package.json` that is derived from the Cargo.toml (and rendered into the "out-dir", previously `pkg/`). While I would believe that there are some advanced `wasm-pack` cases where this above change might be undesirable, for now, the constraint incurred by that transposition is unblocking in other ways. For example, rather than necessitating parallel metadata support in the `Cargo.toml` for every `package.json` property (think about npm specific properties like "labels", "bugs", "private"), we can simply change our `package.json` as we see fit. I am claiming that this side-stepping of version control defined within `Cargo.toml` is acceptable since we do not have any intention of publishing the `query-planner-wasm` package to crates.io on its own. Instead, we will continue to publish the `wasm-pack`'d-from-this-crate `@apollo/query-planner-wasm` package directly to npm's registry. To reflect that desire, I've marked the `query-planner-wasm` package as "private" using the `private = true` property in its `Cargo.toml`. In theory, this change could de-stabilize the work that `wasm-pack` does and that's worth calling out. For example, by not allowing it to generate the `files` property (which indicates which artifacts are emitted into the `npm pack`'d bundle), we might be not allowing it to add other (Future? Unexpected?) important emitted files. I've instead put those `files` directly into the new `package.json` source of truth, along with other appropriate properties, like `types` and `main`. The largest risk here - based on our intended use of the package - seems to be relevant only if we were to rename the host Crate. To demitigate that risk, I've explicitly wired up the `package.json` with `--out-dir` and `--out-file` flags to ensure that it always emits `index`-prefixed files. Furthermore, they are now emitted into the `dist` directory, to be parallel with all our other npm package patterns which do the same. I will note that, `wasm-pack` may address some of the work-arounds here in the future, but best I can tell, they will be roughly compatible with what we're doing here. I am basing this outstanding issues, PRs, RFCs and documentation on the `wasm-pack` project, referenced below. Ref: https://rustwasm.github.io/rfcs/008-npm-dependencies.html#unresolved-questions (See last section) Ref: https://rustwasm.github.io/docs/wasm-pack/commands/build.html#extra-options (See footnote) Ref: rustwasm/wasm-pack#606 Ref: rustwasm/wasm-pack#840 * Update query-planner-wasm/package.json Co-authored-by: Trevor Scheer <trevor@apollographql.com> Co-authored-by: Trevor Scheer <trevor@apollographql.com>
jpgneves
added a commit
to jpgneves/wasm-pack
that referenced
this issue
Mar 5, 2021
jpgneves
added a commit
to jpgneves/wasm-pack
that referenced
this issue
Jul 6, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Specified upstream RFC 8 is a way for NPM dependencies to work transitively throughout the crate graph. The first phase of supporting this is now implemented in wasm-bindgen and the next phase is in
wasm-packitself. Thankfully it shouldn't be too hard!Currently today
wasm-packwill write out apackage.json, but this will overwrite apackage.jsoncreated bywasm-bindgen, if one exists. Instead whatwasm-packshould do is:package.json, first check to see if one already exists at the target locationdependenciesmap, if it exists, otherwise start with an empty objectpackage.jsonagain, overwriting what was previously there.Basically the
package.jsonthatwasm-packwrites should just be merged with thepackage.jsonemitted bywasm-bindgenif it's already there.The text was updated successfully, but these errors were encountered: