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

Add cacheSource repeater property #1961

Closed
wants to merge 3 commits into from
Closed

Add cacheSource repeater property #1961

wants to merge 3 commits into from

Conversation

JustinGeorgi
Copy link
Contributor

fixes #1956

Adds a boolean property to the repeater (and relevant docs), cacheSource which allows the user to determine whether the repeater should refresh the source array with css and object changes or cache the source array for performance improvements.

Signed-off-by : Justin Georgi justin.georgi@gmail.com

Signed-off-by: jgeorgi <justin.georgi@gmail.com>
@JustinGeorgi JustinGeorgi requested a review from a team as a code owner July 12, 2023 05:10
@florian-h05 florian-h05 self-assigned this Jul 13, 2023
Copy link
Contributor

@florian-h05 florian-h05 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good, but I would prefer that even when cacheSource is enabled, only loaded sources are cached. This could be accomplished by just moving the this.sourceCache line into the if blocks for loaded sources.

Signed-off-by: jgeorgi <justin.georgi@gmail.com>
@JustinGeorgi
Copy link
Contributor Author

I would prefer that even when cacheSource is enabled, only loaded sources are cached

That makes good sense now that you point it out. That should all be taken care of.

@florian-h05
Copy link
Contributor

I would prefer to early return when cached, and to store the data in the cache inside the if-clauses instead of returning from there.

I would commit, but I cannot because your main branch is protected, can you please commit that code:

<template>
  <ul v-if="config.listContainer" :class="config.containerClasses" :style="config.containerStyle">
    <generic-widget-component :context="ctx" v-for="(ctx, idx) in childrenContexts" :key="'repeater-' + idx" @command="onCommand" />
  </ul>
  <fragment v-else-if="config.fragment">
    <generic-widget-component :context="ctx" v-for="(ctx, idx) in childrenContexts" :key="'repeater-' + idx" @command="onCommand" />
  </fragment>
  <div v-else :class="config.containerClasses" :style="config.containerStyle">
    <generic-widget-component :context="ctx" v-for="(ctx, idx) in childrenContexts" :key="'repeater-' + idx" @command="onCommand" />
  </div>
</template>

<script>
import mixin from '../widget-mixin'
import { OhRepeaterDefinition } from '@/assets/definitions/widgets/system'
import { compareItems, compareRules } from '@/components/widgets/widget-order'
import { Fragment } from 'vue-fragment'

export default {
  mixins: [mixin],
  components: {
    Fragment
  },
  widget: OhRepeaterDefinition,
  data () {
    return {
      sourceCache: null
    }
  },
  computed: {
    childrenContexts () {
      const iterationContext = (ctx, el, idx, source) => {
        // takes the context with the added variables
        const loopVars = {}
        if (ctx.loop) {
          for (const loopKey in this.context.loop) {
            this.$set(loopVars, loopKey, this.context.loop[loopKey])
          }
        }
        loopVars[this.config.for] = el
        loopVars[this.config.for + '_idx'] = idx
        loopVars[this.config.for + '_source'] = source

        this.$set(ctx, 'loop', loopVars)

        return ctx
      }

      let source = this.source
      if (!Array.isArray(source)) return []

      if (this.config.filter) {
        source = source.filter((el, idx, source) =>
          this.evaluateExpression('filterExpr', '=' + this.config.filter,
            iterationContext(this.childContext(this.context.component), el, idx, source)))
      }
      if (this.config.map) {
        source = source.map((el, idx, source) =>
          this.evaluateExpression('mapExpr', '=' + this.config.map,
            iterationContext(this.childContext(this.context.component), el, idx, source)))
      }

      let contexts = []
      let idx = 0
      for (let i of source) {
        contexts.push(...this.context.component.slots.default.map((c) => {
          return iterationContext(this.childContext(c), i, idx, source)
        }))

        idx++
      }

      return contexts
    }
  },
  asyncComputed: {
    source () {
      if (this.config.cacheSource && this.sourceCache) return this.sourceCache
      let sourceResult
      if (this.config.sourceType === 'range') {
        const start = this.config.rangeStart || 0
        const stop = this.config.rangeStop || 10
        const step = this.config.rangeStep || 1
        sourceResult = Promise.resolve(Array(Math.ceil((stop + 1 - start) / step)).fill(start).map((x, y) => x + y * step))
      } else if (this.config.sourceType === 'itemsWithTags' && this.config.itemTags) {
        sourceResult = this.$oh.api.get('/rest/items?metadata=' + this.config.fetchMetadata + '&tags=' + this.config.itemTags).then((d) => Promise.resolve(d.sort(compareItems)))
        this.sourceCache = (this.config.cacheSource) ? sourceResult : null
      } else if (this.config.sourceType === 'itemsInGroup') {
        sourceResult = this.$oh.api.get('/rest/items/' + this.config.groupItem + '?metadata=' + this.config.fetchMetadata + '&tags=' + this.config.itemTags).then((i) => Promise.resolve(i.members.sort(compareItems)))
        this.sourceCache = (this.config.cacheSource) ? sourceResult : null
      } else if (this.config.sourceType === 'itemStateOptions') {
        sourceResult = this.$oh.api.get('/rest/items/' + this.config.itemOptions).then((i) => Promise.resolve((i.stateDescription) ? i.stateDescription.options : []))
        this.sourceCache = (this.config.cacheSource) ? sourceResult : null
      } else if (this.config.sourceType === 'itemCommandOptions') {
        sourceResult = this.$oh.api.get('/rest/items/' + this.config.itemOptions).then((i) => Promise.resolve((i.commandDescription) ? i.commandDescription.commandOptions : []))
        this.sourceCache = (this.config.cacheSource) ? sourceResult : null
      } else if (this.config.sourceType === 'rulesWithTags' && this.config.ruleTags) {
        sourceResult = this.$oh.api.get('/rest/rules?summary=true' + '&tags=' + this.config.ruleTags).then((r) => Promise.resolve(r.sort(compareRules)))
        this.sourceCache = (this.config.cacheSource) ? sourceResult : null
      } else {
        sourceResult = Promise.resolve(this.config.in)
      }
      return sourceResult
    }
  }
}
</script>

Signed-off-by: jgeorgi <justin.georgi@gmail.com>
@JustinGeorgi
Copy link
Contributor Author

JustinGeorgi commented Jul 14, 2023

I would prefer to early return when cached, and to store the data in the cache inside the if-clauses instead of returning from there.

Sorry, I misintrepreted your previous request 🤣. It seemed strange to me that you were asking to move the early return into the if blocks, but figured you knew something I didn't. This should put it back right with the caching in the if blocks.

edit: Also I've invited you the pr repo if you need to change anything else. I'm about to get on a transoceanic flight.

@florian-h05
Copy link
Contributor

Sorry, I misintrepreted your previous request 🤣.

Oops, sorry 🙈

Also I've invited you the pr repo if you need to change anything else. I'm about to get on a transoceanic flight.

Thanks, I think the problem is that you created the PR from your main and not a feature branch. I usually don't have any problems writing to PR branches. I will take care of the linting failure and then merge this.

@florian-h05
Copy link
Contributor

@JustinGeorgi
Even as a collaborator I cannot commit because you enabled branch protection for your main branch.
Due to lack of time, I'll open a new PR to the openHAB repo from my branch and merge this one.

@florian-h05
Copy link
Contributor

Superseded by #1963.

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.

Repeater static values vs dynamic refresh
2 participants