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

After merge of Tasks 1.9.0 code, tasks results will not refresh if users edit tasks-x specific fields in Task object #19

Open
claremacrae opened this issue Aug 2, 2022 · 2 comments

Comments

@claremacrae
Copy link
Contributor

Summary

  • If you have added any fields to Task in your fork (like date created) ...
  • ... and you merge any post 1.9.0 Tasks code in to your fork ...
  • ... as things currently stand, if a user edits a task and changes a value of one of those new fields ...
  • ... then any tasks code blocks (and probably SQL code blocks) in any active files will not be refreshed.

More info below.

Background

This is to expand on my message in Discord on 2022-07-18

@sytone please don’t merge 1.9.0 to your fork yet. I need to generalise one of the performance fixes so that it works when there are more values in a Task (like your Added field). Will let you know when it’s ready.

If you merge now, user task blocks won’t be updated if they edit or add Added values…

The context is PR #894 in obsidian-tasks.

That PR describes thoroughly the intention of the change.

The specific code

And these new chunks of code:

Task.identicalTo():

obsidian-tasks/Task.ts at cad0391d2a3267eb3682a012d2dded3c0f0e4c98 · obsidian-tasks-group/obsidian-tasks · GitHub

Task.test.ts - 'identicalTo'

obsidian-tasks/Task.test.ts at cad0391d2a3267eb3682a012d2dded3c0f0e4c98 · obsidian-tasks-group/obsidian-tasks · GitHub

Action required

  1. Either your fork of Task.identicalTo() needs to be edited to add checks (and tests, of course) for any new fields you have added
  2. Or the existing Task.identicalTo() needs to re-implemented so that it checks all fields in Task, recognises their type, and applies the correct identity-check.

Reasoning: as schemar noted in this code review comment:

perf: Stop many non-task edits triggering a redraw of all active tasks blocks by claremacrae · Pull Request #894 · obsidian-tasks-group/obsidian-tasks · GitHub

The challenge with this implementation (Task.identicalTo()) is that you now need to remember to update it if you change the structure of a task. For example when you add a new field to it.

It's especially difficult to remember, as all the date fields are declared further down and not necessarily visible when looking at the function head.

Is there good documentation on what to do to add a new field to Task? (or possibly other modifications)
Could there be a way where you iterate over the properties of the objects (e.g. Object.keys()) and compare based on the type of the value at runtime (e.g. instanceof)?

If this is not possible, maybe document here why it isn't possible.

@claremacrae
Copy link
Contributor Author

I have been planning to improve Task.identicalTo() ever since receiving that code review comment.

However, I've been in the happy position that new high-quality PRs contributions have been coming in to Tasks as fast as I can keep up with them, so I have not got to it yet.

If you are able to come up with a generalised version of Task.identicalTo() I would be very happy to back-port it to the original Tasks code.

@sytone
Copy link
Owner

sytone commented Aug 2, 2022

Yeah, I saw the Identical check in the changes in the main Tasks branch and have updated. One way to do it may be to use decorators in TS and then reflect of those to do the identical check. That way only a decorator needs to be added to the property for it to be covered in the identical check.

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