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

Missing property type in reversed remove op #10

Open
lukasschwab opened this issue May 5, 2022 · 1 comment
Open

Missing property type in reversed remove op #10

lukasschwab opened this issue May 5, 2022 · 1 comment

Comments

@lukasschwab
Copy link

Description

This demo mentioned in README.md seems broken:

Compile back from an updated "simple issue" to a new github issue file:

cat ./demo/simple-issue.json | node ./dist/cli.js -l ./demo/github-arthropod.lens.yml -r -b ./demo/github-issue.json

In practice:

$ cat ./demo/simple-issue.json | node ./dist/cli.js -l ./demo/github-arthropod.lens.yml -r -b ./demo/github-issue.json
./dist/json-schema.js:35
        throw new Error(`Missing property name in addProperty.\nFound:\n${JSON.stringify(property)}`);
        ^

Error: Missing property name in addProperty.
Found:
{"op":"add","name":"labels"}
    at addProperty (./dist/json-schema.js:35:15)
    at applyLensOperation (./dist/json-schema.js:338:20)
    at ./cambria-project/dist/json-schema.js:366:16
    at Array.reduce (<anonymous>)
    at Object.updateSchema (./dist/json-schema.js:363:17)
    at Object.applyLensToDoc (./dist/doc.js:51:40)
    at Object.<anonymous> (./dist/cli.js:25:22)
    at Module._compile (internal/modules/cjs/loader.js:1085:14)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1114:10)
    at Module.load (internal/modules/cjs/loader.js:950:32)

Cause

github-arthropod.lens.yml defines a valid remove op––no type required:

- remove:
name: labels

And reverse.ts just swaps in the add op:

case 'remove':
return {
...lensOp,
op: 'add',
}

But an add op must specify a type! It's the type missing, not the name:

if (!name || !type) {
throw new Error(`Missing property name in addProperty.\nFound:\n${JSON.stringify(property)}`)
}

Solutions

  • Require types on remove ops.
  • Set an unrestrictive default type when reversing a remove op without a type. I sketched that out here, and confirmed the demo works as expected and unit tests pass: lukasschwab@77b602e
  • Loosen the commitment to reversibility. This seems incompatible with the project goals.

Let me know if you'd like me to open a PR.

Cool stuff––enjoyed the HYTRADBOI talk!

@michielbdejong
Copy link

I ran into this same problem when trying to follow the readme instructions, and looks like lukasschwab@77b602e indeed makes this error disappear.

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

No branches or pull requests

2 participants