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

graph: fix adding nameless specs #241

Merged
merged 2 commits into from
Nov 26, 2024
Merged

Conversation

ruyadorno
Copy link
Member

@ruyadorno ruyadorno commented Nov 17, 2024

This PR fixes adding nameless dependency specs when using the cli
e.g: vlt install github:foo/bar along with fixing inconsistencies
when merging cli-defined dependencies with manifest-defined values.

A few connected changes found when turning many stones related to this
problem that are worthy of mention:

  • Graph.removeNode removes the edge from its from node instead of
    just unsetting the to value of that edge.
  • append-nodes will fix up and update entries in the user-provided
    add values, so that we can add a proper dependency name when writing
    to that importer package.json at the end of reify.
  • cleaned up the logic between ideal.build and its
    get-importer-specs module so values should be properly merged when
    adding / removing new dependencies.
  • Make lockfile load a little more resilient by making sure it does not
    throw when trying to deconstruct values from a missing options
    property.
  • updated a few tests and snapshots that had either wrong assumptions
    and / or bad fixture data

Copy link

vercel bot commented Nov 17, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
docs.vlt.sh ⬜️ Ignored (Inspect) Visit Preview Nov 22, 2024 6:51pm

This changeset fixes adding nameless dependency specs to the cli
e.g: `vlt install github:foo/bar` along with fixing inconsistencies
when merging cli-defined dependencies with manifest-defined values.

A few connected changes found when turning many stones related to this
problem that are worthy of mention:

- `Graph.removeNode` removes the edge from its `from` node instead of
  just unsetting the `to` value of that edge.
- `append-nodes` will fix up and update entries in the user-provided
  `add` values, so that we can add a proper dependency name when writing
  to that importer `package.json` at the end of reify.
- cleaned up the logic between `ideal.build` and its
  `get-importer-specs` module so values should be properly merged when
  adding / removing new dependencies.
- updated a few tests and snapshots that had either wrong assumptions
  and / or bad fixture data
Make lockfile load a little more resilient by making sure it does not
throw when trying to deconstruct values from a missing `options`
property.

Even though it should never be the case, we still had some internal
tests that were confused by that, not to mention it might be a user
surface that benefits from the extra resiliency despite not being a file
that should be manually changed.
@ruyadorno ruyadorno force-pushed the ruyadorno/fix-adding-nameless-specs branch from 55110d2 to 663abf8 Compare November 22, 2024 18:51
@isaacs isaacs merged commit 663abf8 into main Nov 26, 2024
5 checks passed
@isaacs isaacs deleted the ruyadorno/fix-adding-nameless-specs branch November 26, 2024 20:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants