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

Feature request: update row #5

Open
DJSundog opened this issue Nov 2, 2020 · 4 comments
Open

Feature request: update row #5

DJSundog opened this issue Nov 2, 2020 · 4 comments

Comments

@DJSundog
Copy link

DJSundog commented Nov 2, 2020

Use case:

If we assume a JSDB table initialized as

db.cars = [
  { make: "Mazda", model: "Miato", color: "red", year: 1992 },
  { make: "GMC", model:" Timmy", color: "black", year: 1987 },
  { make: "Ford", model: "F5", color": blue", year: 1987 }
]

I would like to be able to update data in the table, either by querying for row index/indices, like

let editRowIndex = db.cars.where('model').is('Miato').getFirstIndex()
// editRowIndex now equals 0

let manyRowIndices = db.cars.where('year').is(1987).getIndices()
// manyRowIndices now equals [1, 2]

or by having access to an update method, like

let result = db.cars.where('model').is('F5').getFirst().update({ model: 'F150' });
// result is true if data was successfully persisted after merging updated fields into db.cars[2]

let multiresult = db.cars.where('year').isLessThan(2000).get().update({ needsSmogTest: true });
// multiresult is true if all matching rows were successfully persisted

As an alternative to the second version, I'd be fine with having the update(newData) method hanging off the query pre-get (so, db.cars.where('model').is('F5').updateFirst(newData) and the like.

Thoughts?

@aral
Copy link
Contributor

aral commented Nov 9, 2020

Hey @DJSundog, you already can! The returned results are proxies and are thus reactive :)

Try:

db.cars.where('model').is('F5').getFirst().model = 'F150'

db.cars.where('year').isLessThan(2000).get().forEach(car => car.needsSmogTest = true)

For the first one, though, I’d write it more defensively as:

const theFirstF5 = db.cars.where('model').is('F5').getFirst()
if (theFirstF5) theFirstF5.model = 'F150'

If we wanted to avoid having to write that out in two lines (i.e., support chaining when there is no result, ala optional chaining) we could modify the behaviour of getFirst(), getLast(), etc., so that they return an empty object when there is no result instead of undefined as they do now*.

So, instead of throwing, the assignment would simply set the model property on a simple empty object, thereby effectively ignoring it.

This would also be consistent with how the results of get() are handled (where an empty array is returned when there is no result).

Thoughts? :)


  • In Node v14+, you can also use the nullish coalescing operator for this:
(db.cars.where('model').is('F5').getFirst() ?? {}).model = 'F150'

@aral
Copy link
Contributor

aral commented Nov 9, 2020

Also, this makes me realise that we need a section along the lines of “Updating data” in the readme.

I’ll wait for your thoughts and then possibly open two internal issues regarding this:

  • getFirst(), getLast(), etc. should return empty object instead of undefined to support method chaining on updates
  • Document how to update data (including how to update data based on the results of queries, which are reactive)

@DJSundog
Copy link
Author

DJSundog commented Nov 9, 2020

Hey, thanks - I just flipped some of my code around and can confirm that it works as you describe (not that I am surprised or anything). That definitely makes things easier!

I think returning undefined on getFirst() et al is semantically nicer than returning an empty object, so that a check for undefined acts as "no such record", but I suppose checking for the truthiness of any property of the returned object would be semantically the same for most use cases.

Adding more docs about the results being reactive would be welcome - I was mistakenly assuming I'd need to write modified data back to the table manually, so this is much nicer - thanks!

@aral
Copy link
Contributor

aral commented Nov 10, 2020

Tracking internally at:

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