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

removeFile in QFile actually expects a test function instead of a File #15727

Closed
IlCallo opened this issue Apr 18, 2023 · 3 comments
Closed

removeFile in QFile actually expects a test function instead of a File #15727

IlCallo opened this issue Apr 18, 2023 · 3 comments
Labels
area/components bug/1-repro-available A reproduction is available and needs to be confirmed. flavour/quasar-cli-webpack kind/bug 🐞 Qv1 Qv2 🔝 Quasar v2 issues

Comments

@IlCallo
Copy link
Member

IlCallo commented Apr 18, 2023

What happened?

Using qFileRef.removeFile(file) won't work, as it actually expects a function

What did you expect to happen?

It should work as per docs API

image

Reproduction URL

https://stackblitz.com/edit/quasarframework-webpack-1u1zmr?file=src/pages/IndexPage.vue

How to reproduce?

  1. Upload a dummy file
  2. Notice 2 buttons appeared on the screen
  3. Open dev tools on console
  4. Click on the first button
  5. Notice the error in the console
  6. Click on the second button
  7. Notice how the file is correctly removed

Flavour

Quasar CLI with Webpack (@quasar/cli | @quasar/app-webpack)

Areas

Components (quasar)

Platforms/Browsers

No response

Quasar info output

No response

Relevant log output

Uncaught TypeError: #<File> is not a function
    at Array.findIndex (<anonymous>)
    at Proxy.removeFile (QFile.js?7d53:117:1)
    at $setup.fileToUpload.onClick._cache.<computed>._cache.<computed> (IndexPage.vue?c8e0:7:1)
    at callWithErrorHandling (runtime-core.esm-bundler.js?f781:155:1)
    at callWithAsyncErrorHandling (runtime-core.esm-bundler.js?f781:164:1)
    at emit$1 (runtime-core.esm-bundler.js?f781:718:1)
    at eval (runtime-core.esm-bundler.js?f781:7232:1)
    at onClick (QBtn.js?9c40:148:1)
    at callWithErrorHandling (runtime-core.esm-bundler.js?f781:155:1)
    at callWithAsyncErrorHandling (runtime-core.esm-bundler.js?f781:164:1)
removeFile @ QFile.js?7d53:117
$setup.fileToUpload.onClick._cache.<computed>._cache.<computed> @ IndexPage.vue?c8e0:7
callWithErrorHandling @ runtime-core.esm-bundler.js?f781:155
callWithAsyncErrorHandling @ runtime-core.esm-bundler.js?f781:164
emit$1 @ runtime-core.esm-bundler.js?f781:718
eval @ runtime-core.esm-bundler.js?f781:7232
onClick @ QBtn.js?9c40:148
callWithErrorHandling @ runtime-core.esm-bundler.js?f781:155
callWithAsyncErrorHandling @ runtime-core.esm-bundler.js?f781:164
invoker @ runtime-dom.esm-bundler.js?9cec:366

Additional context

Looking at the commit history, it appears the problem is there since the QFile inception
Since we internally use removeAtIndex and there are no examples using this method on the docs, this never got caught

removeFile expects a File as param per docs, but then uses it as a function provided to findIndex

const index = innerValue.value.findIndex(file)

I'd propose to fix it in this way

// TODO: (Qv3) deprecate function signature and remove it in next major version
const index = innerValue.value.findIndex(file instanceof Function ? file : fileInArray => fileInArray === file)

The file instanceof Function is meant to avoid a breaking change for who's using the current incorrect signature, which should probably be deprecated and removed in the next major version

The same problem affects Qv1 and the fix can be backported

The same function on QUploader works correctly, accepting a File as documented in the docs

@IlCallo IlCallo added kind/bug 🐞 Qv2 🔝 Quasar v2 issues labels Apr 18, 2023
@github-actions github-actions bot added area/components bug/1-repro-available A reproduction is available and needs to be confirmed. flavour/quasar-cli-webpack labels Apr 18, 2023
@IlCallo IlCallo added the Qv1 label Apr 18, 2023
@pdanpdan
Copy link
Collaborator

The code should have been indexOf

@IlCallo
Copy link
Member Author

IlCallo commented Apr 18, 2023

Yeah, that makes sense too

Backward compatible using that would be

// TODO: (Qv3) deprecate function signature and remove it in next major version
const index = file instanceof Function ? innerValue.value.findIndex(file) : innerValue.value.indexOf(file)

@rstoenescu
Copy link
Member

Fixed in 2.12.0 and 1.22.10

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/components bug/1-repro-available A reproduction is available and needs to be confirmed. flavour/quasar-cli-webpack kind/bug 🐞 Qv1 Qv2 🔝 Quasar v2 issues
Projects
None yet
Development

No branches or pull requests

3 participants