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

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

Merged
merged 3 commits into from Apr 29, 2024

Conversation

wesleybl
Copy link
Member

The function works without these parameters. Avoid error:

TS2554: Expected 5 arguments, but got 1.

fixes: #5985

Copy link

netlify bot commented Apr 26, 2024

Deploy Preview for volto failed.

Name Link
🔨 Latest commit b160574
🔍 Latest deploy log https://app.netlify.com/sites/volto/deploys/662f96b05d0b36000895b104

Copy link

netlify bot commented Apr 26, 2024

Deploy Preview for plone-components canceled.

Name Link
🔨 Latest commit b160574
🔍 Latest deploy log https://app.netlify.com/sites/plone-components/deploys/662f96b0234ad100086cd2b9

@sneridagh
Copy link
Member

@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

"build:types": "tsc --project tsconfig.declarations.json",

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
Copy link
Member

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

@sneridagh
Copy link
Member

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

@sneridagh
Copy link
Member

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

@wesleybl
Copy link
Member Author

@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!

@sneridagh sneridagh merged commit 6e3341e into main Apr 29, 2024
69 of 73 checks passed
@sneridagh sneridagh deleted the asyncconnect branch April 29, 2024 15:47
sneridagh added a commit that referenced this pull request Apr 29, 2024
* main: (49 commits)
  Update to Plone 6.0.11 (#5989)
  Defines the last 4 parameters of the `asyncConnect` function with optional (#5986)
  Flexibilize the pins for all ESlint deps, in Volto and generators (#5991)
  Release 18.0.0-alpha.29
  Release @plone/types 1.0.0-alpha.11
  fix: pass down locale to IntlProvider (#5976)
  Add Vite (client only, no SSR) build. Update Next.js 14.2.2 and Remix to 2.8.0 (#5970)
  Fix no router link in logo (#5981)
  Improve postinstall script, building all the deps (#5980)
  Better BlocksData types (#5979)
  Add missing types ts fields
  Release 18.0.0-alpha.28
  Release @plone/slate 18.0.0-alpha.12
  Release @plone/registry 1.5.6
  Improvements to the monorepo setup with utilities, especially ESLint.… (#5969)
  DEV: put nvm installation section into a separate include file (#5968)
  Bundle optimization (#5295)
  [client] Move provider to own package - Use parcel - `@plone/providers` (#5887)
  Fix flaky test 'As editor I can add links' by using getSlateEditorAndType (#5966)
  Fix rendering if ConditionalLink has no children (#5963)
  ...
sneridagh added a commit that referenced this pull request May 7, 2024
* main: (1206 commits)
  Allow Makefile to be modified by Makefile.local if present (#5997)
  Cypress replaced wait with assertions (#5998)
  Replace spinner with Progress bar and fix it's position . (#5620) (#5929)
  Fix type for component registry components (#6002)
  Fix 301 and 302 redirects. (#5999)
  Release 18.0.0-alpha.30
  Force add .d.ts files to releaser
  Release generate-volto 9.0.0-alpha.15
  Fix server side sidebar rendering (#5993)
  Cleaned up useless injectIntl in DefaultView (#5995)
  Fix image disappears after pressing the Enter key on title field in i… (#5975)
  Update to Plone 6.0.11 (#5989)
  Defines the last 4 parameters of the `asyncConnect` function with optional (#5986)
  Flexibilize the pins for all ESlint deps, in Volto and generators (#5991)
  Release 18.0.0-alpha.29
  Release @plone/types 1.0.0-alpha.11
  fix: pass down locale to IntlProvider (#5976)
  Add Vite (client only, no SSR) build. Update Next.js 14.2.2 and Remix to 2.8.0 (#5970)
  Fix no router link in logo (#5981)
  Improve postinstall script, building all the deps (#5980)
  ...
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.

Wrong parameters in the asyncConnect function
3 participants