quasarframework / quasar

Quasar Framework - Build high-performance VueJS user interfaces in record time
https://quasar.dev
MIT License
25.29k stars 3.43k forks source link

QTable :rows does not accept undefined anymore #17058

Closed nschermer closed 1 month ago

nschermer commented 1 month ago

What happened?

With update to quasar 2.15.2, QTable does not accept undefined as :rows anymore.

What did you expect to happen?

This used to work,

Reproduction URL

https://jsfiddle.net/b4hyesmw/3/

How to reproduce?

send undefined to :rows property

Flavour

Quasar CLI with Vite (@quasar/cli | @quasar/app-vite)

Areas

Components (quasar)

Platforms/Browsers

Firefox

Quasar info output

Operating System - Windows_NT(10.0.22631) - win32/x64
NodeJs - 20.11.0

Global packages
  NPM - 10.5.0
  yarn - Not installed
  @quasar/cli - 2.4.0
  @quasar/icongenie - Not installed
  cordova - Not installed

Important local packages
  quasar - 2.15.2 -- Build high-performance VueJS user interfaces (SPA, PWA, SSR, Mobile and Desktop) in record time
  @quasar/app-vite - 2.0.0-beta.5 -- Quasar Framework App CLI with Vite
  @quasar/extras - 1.16.11 -- Quasar Framework fonts, icons and animations
  eslint-plugin-quasar - Not installed
  vue - 3.4.21 -- The progressive JavaScript framework for building modern web UI.
  vue-router - 4.3.0
  pinia - 2.1.7 -- Intuitive, type safe and flexible Store for Vue
  vuex - Not installed
  vite - 5.2.7 -- Native-ESM powered web dev build tool
  vite-plugin-checker - Not installed
  eslint - 8.57.0 -- An AST-based pattern checker for JavaScript.
  esbuild - 0.20.2 -- An extremely fast JavaScript and CSS bundler and minifier.
  typescript - 5.3.3 -- TypeScript is a language for application scale JavaScript development
  workbox-build - Not installed
  register-service-worker - 1.7.2 -- Script for registering service worker, with hooks
  electron - Not installed
  electron-packager - Not installed
  electron-builder - Not installed
  @capacitor/core - Not installed
  @capacitor/cli - Not installed
  @capacitor/android - Not installed
  @capacitor/ios - Not installed

Relevant log output

Uncaught (in promise) TypeError: computedRows.value is undefined
    getTBody quasar.esm.js:25369
    getBody quasar.esm.js:25266
    setup quasar.esm.js:25812
    renderComponentRoot runtime-core.esm-bundler.js:877
    componentUpdateFn runtime-core.esm-bundler.js:6004
    run reactivity.esm-bundler.js:177
    update runtime-core.esm-bundler.js:6135

Additional context

No response

rstoenescu commented 1 month ago

Hi,

Yes, the rows prop is now required. This is because it doesn't makes sense not to have it.

justinross commented 1 month ago

I can't seem to find Quasar's semver policy, but should this qualify as a 'breaking change' for the purposes of semantic versioning? It definitely doesn't make sense for the rows prop to be undefined, but since it worked (or at least didn't throw an error) before and throws an error now, I'd argue that, strictly-speaking, a change like this should wait for a major release, however silly that seems in practice.

Also, is there somewhere changes like this are listed? I can't seem to find it documented anywhere.

Thanks!

JacksonBowe commented 3 weeks ago

I load my table data via tanstack-query. API request to grab data. On initial page load the :rows props is undefined. I also think this should have been marked as a breaking change.

rstoenescu commented 3 weeks ago

Use an empty array instead of undefined. This is a very old bug, but a bug nonetheless. It makes absolutely no sense to render a QTable with no rows. The rows are the bread and butter of QTable.

justinross commented 3 weeks ago

I don’t think you’ll find anyone disagreeing with the need for the fix in general. It’s an easy issue to avoid, by using QTable correctly (and, in my codebase at least, was easy enough to fix). I think I’m just requesting that similar changes in the future at least be mentioned in changelogs as potentially breaking.

Even bug fixes can be breaking changes, as silly as that may be.

Anyway, thanks for listening. Still love the framework of course. 😁

rstoenescu commented 3 weeks ago

We have done some minor changes in the past that can be considered as legit breaking changes (but again, very small ones, almost "insignificant"). We post such things in the release notes: here or better yet here.

In any case, thanks for understand and enjoy Quasar!

jbwl commented 2 days ago

Use an empty array instead of undefined. This is a very old bug, but a bug nonetheless. It makes absolutely no sense to render a QTable with no rows. The rows are the bread and butter of QTable.

I understand this from a clean code point of view, but as others pointed out, when the row data is not available yet (ie. loading is true), it doesn't make sense to have it set to anything else but undefined. Setting it to an empty array before arrival would not be logical because it would be the same state as "no data" but at that point we do not know if there really is no data.

It used to work well to set loading to true while the data is being fetched and then set the rows to the fetched data. If I have to use v-if="data" on Q-Table, the whole table won't render instead of at least showing the loading bar.

Maybe there is a more elegant solution I'm not thinking about, but I have to agree that the table data being undefined (for XHR data while loading) makes absolutely sense and is intuitive.

rstoenescu commented 2 days ago

Under the covers, that's what QTable did anyways. It set rows to an empty array when the prop was missing.

From a clean code point of view, that's why there's the "loading" prop, to signal that data is coming and current rows "don't matter".