search

plone / volto

React-based frontend for the Plone Content Management System
https://demo.plone.org/
MIT License
427 stars 574 forks source link

Defines the last 4 parameters of the `asyncConnect` function with optional #5986

Closed wesleybl closed 1 week ago

wesleybl commented 1 week ago

The function works without these parameters. Avoid error:

TS2554: Expected 5 arguments, but got 1.

fixes: #5985

netlify[bot] commented 1 week ago

Deploy Preview for volto failed.

Name Link
Latest commit b160574f5ebf0cb927d10610cd5c2ed6e2e64b6c
Latest deploy log https://app.netlify.com/sites/volto/deploys/662f96b05d0b36000895b104
netlify[bot] commented 1 week ago

Deploy Preview for plone-components canceled.

Name Link
Latest commit b160574f5ebf0cb927d10610cd5c2ed6e2e64b6c
Latest deploy log https://app.netlify.com/sites/plone-components/deploys/662f96b0234ad100086cd2b9
sneridagh commented 1 week ago

@wesleybl unfortunately, we cannot do this :( The types in packages/volto/types are autogenerated, given the information tsc can infer from the code and JSDoc annotations in the .js code. See https://github.com/plone/volto/blob/e7d6f1c0be483c6560a552bae615b3f80ec21c1e/packages/volto/package.json#L42

This was done for convenience, because the prior situation was far worse, when coming to TS support and VSCode autocompletion and "go to reference" matters. Problem is that any change will be overwritten afterwards.

What we can do is to improve the JSDocs and hints in the code itself, so the generated types are improved. @wesleybl could you try this approach instead, then run the build:types command and see if that improves?

Unfortunately, we will have to bear with this kind of things until the vast majority of our code base is not TS-based. :(

sneridagh commented 1 week ago

@wesleybl ok, now I read more carefuly, so your addon tests are failing because of this.

sneridagh commented 1 week ago

@wesleybl could you please share the test that cause the failure? Maybe we can add them to the test batteries in CI.

sneridagh commented 1 week ago

@tiberiuichim could you please check the typings? I want to make sure that we are not messing this up.

wesleybl commented 1 week ago

@wesleybl could you please share the test that cause the failure? Maybe we can add them to the test batteries in CI.

@sneridagh It is not a specific test. I think any test that import:

https://github.com/plone/volto/blob/e7d6f1c0be483c6560a552bae615b3f80ec21c1e/packages/volto/src/components/manage/Controlpanels/index.tsx

will give the error. I commented on the test that failed and another test started to fail.

The types in packages/volto/types are autogenerated, given the information tsc can infer from the code and JSDoc annotations in the .js code

After your commit, the parameters became optional, even after running build:types. So I think it's ok now. Thanks!