Quasar: qDialog Global TypeDef lacks of definitions for custom props

Created on 18 Jun 2019  路  17Comments  路  Source: quasarframework/quasar

Describe the bug
image

The type definitions lack for QVueGlobals['dialog'] lacks definitions for the custom props for components using the Custom Component QDialog Evoking.

To Reproduce
Any custom component qdialog envoking using custom props.
this.$q.dialog({ component: CustomDialogComponent, root: this.$root, customProps: { anyProp: prop } }

Expected behavior
No errors on typescript language server.

Platform (please complete the following information):
OS: Windows 10
Node: 10
NPM: 6.9.0
Yarn: 1.16.0
Browsers: Chrome 75.0

bug

Most helpful comment

I believe that this is the best approach and should be documented as such. I avoid any breaking changes to the current api and provides a fully type safe way of passing the options.

All 17 comments

Extra: it should include a [propsName: string]: any inside opts object from dialog, as it is auto-generated by the build script I'm by-passing it including this line in the index.d.ts inside the dist folder.

4498 - made this PR to solve the problem for now, until some new mechanics is applied to the build script.

Does your suggest PR resolve the issue completely?
Otherwise I suggest generating a named Type interface QDialogOptions. This would make the QDialog type def look something like

export interface Dialog {
    create(opts : QDialogOptions)

This would then support Type Declaration Merging, which the same mechanism use by Vuejs to extend the VueConstructor. This would provide a consistent approach to extending the Typings. An example would need to be added to the documentation to explain this mechanism and how it can be used.

Well, I'm working in my personal project using this modification and it is working beautifully.

I think your approach is smarter and makes easy for extending types, but once we extend this type inside a project it gonna get extended for the entire project. In my point of view, it's going to pollute the typedef for QDialog. What you want can be done defining a new component module extending the QDialog interface, then it will not pollute the typedef and will work in the way you are thinking.

In my PR I'm assuming you can pass any kind of prop to the component, thus you can make custom components extending QDialog interface having your own props.

I considered that would be a potential issue, as it is global change to the type def. The change itself that I'm making should be made anyway as the feature is only supported for a methods returning type, not a parameter's type. This does not prevent your suggest PR from being made as well. I will discuss the options which Razvan and get his thoughts, and get back to you.

@marco-quintella as discussed I created a PR to generate an interface QDialogOptions. I will discuss both PRs with Razvan and recommend a way forward that most likely includes both.

@outofmemoryagain It's ok. I'm a big fan of quasar and it makes my job a lot easier, any improvement it's always welcome. I had a look in your commit, I think improves not only matter in fact now but opens a way to reuse the interfaces generated by the TS build. In my mind, using quasar should be simple and keep that line of vue, a clear path, if understand me. Probably will be easier to develop with quasar if we can do something like:

this.$q.dialog({
  component: CustomComponent,
  props: anyProps
} as MyCustomInterface)

interface MyCustomInterface extends QDialogOptions {
  aProp: aType,
  ...
}

For me, it would be an easier way to keep the type safety and preserve the neat aspect of the plugin.

By the way, I think there is a missing typedef on the validation of the input and select components. This week I used it with the QForm and then I realized that computed hasError is not in the typedef. Should I address a new issue for that? Or you guys want to handle this together with this QDialog issue?

Looking at the hasError issue now.

@marco-quintella as you have noticed the Type definitions are generated based on the content found in the API json files. These files are used for a few things during the build process, Type Generation and Documentation being two of them. Your PR adds an Index Property and it does fix the issue enabling passing of non defined properties as part of the options argument, but it would also impact the documentation as well. I have some thoughts on how do it without impacting the generated docs. I don't have time today to address it, but I will try go get something in place soon.

On the hasError front after digging into it, currently we do not have any computed properties documented as part of the API json files. This will result in any of those properties not being in the docs or the type definitions. I'm proposing that they be added, but it may take a little time for that to be put in place, as some of the build scripts will need to be adjusted and the JSON files updated. Can you post a separate issue for hasError issue so we can track this one separately?

Also your suggestion above works with the current API with a slight change to your code...

interface MyCustomInterface extends QDialogOptions {
  aProp: string
  ...
}
...
this.$q.dialog({
  component: CustomComponent,
  aProp: "aprop val"
} as MyCustomInterface )

I believe that this is the best approach and should be documented as such. I avoid any breaking changes to the current api and provides a fully type safe way of passing the options.

Ok, I gonna dig up a little bit in the hasError, cause its been mentioned in the documentation examples as a way to validate the inputs when you don't use a qForm. But somehow it doesn't appear in the api documentation. In my opinion, it should be there.

Also your suggestion above works with the current API with a slight change to your code...

interface MyCustomInterface extends QDialogOptions {
  aProp: string
  ...
}
...
this.$q.dialog({
  component: CustomComponent,
  aProp: "aprop val"
} as MyCustomInterface )

I am testing it now in my project.

I am aware of the hasError issue and how to fix both the types and API docs. I just need to get some internal agreement on the approach.

If you can create a new issue that focuses on both the API and Type issue with hasError, it will help me discuss the solution with the right people

@marco-quintella were you able to verify that this resolved your issue with the Custom component properties? If so can you please close the issue?

Issue solved! Using the model:

interface MyCustomInterface extends QDialogOptions {
  aProp: string
  ...
}
...
this.$q.dialog({
  component: CustomComponent,
  aProp: "aprop val"
} as MyCustomInterface )
Was this page helpful?
0 / 5 - 0 ratings

Related issues

danikane picture danikane  路  3Comments

jean-moldovan picture jean-moldovan  路  3Comments

mesqueeb picture mesqueeb  路  3Comments

tombarfoot picture tombarfoot  路  3Comments

green-mike picture green-mike  路  3Comments