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

Should @plone/types be a dependency on Volto? #5870

Closed
wesleybl opened this issue Mar 12, 2024 · 7 comments · Fixed by #5872
Closed

Should @plone/types be a dependency on Volto? #5870

wesleybl opened this issue Mar 12, 2024 · 7 comments · Fixed by #5872

Comments

@wesleybl
Copy link
Member

Describe the bug
I updated an app for Volto 18.0.0-alpha.18 and started receiving the error below in my tests:

 Test suite failed to run
    node_modules/@plone/volto/src/helpers/Slots/index.tsx:1:30 - error TS2307: Cannot find module '@plone/types' or its corresponding type declarations.
    1 import type { Content } from '@plone/types';

@plone/types it is a development dependency on Volto. I wonder if it shouldn't be a real dependency. Or should I place it as a development dependency on my addon?

To Reproduce
Steps to reproduce the behavior:

  1. Create an app with Volto 18.0.0-alpha.18
  2. Create an addon.
  3. Create a block in addon.
  4. Create a test of edit block.
  5. run CI=true NODE_ICU_DATA=node_modules/full-icu yarn test --coverage --maxWorkers=2

Expected behavior
test run without error.

Software (please complete the following information):

  • Volto Version: 18.0.0-alpha.18
@wesleybl
Copy link
Member Author

Hmm, this error does not occur locally only on the CI. I have the build job that runs yarn install and I pass the node_modules folder via artifacts to the test job. Do I have to pass any more folders?

@wesleybl
Copy link
Member Author

I managed to simulate the error locally. I deleted the repository, made a new clone and ran the tests. Then the error occurred. When I ran the tests a second time, the errors no longer occurred. Then something happened on the first run that caused the error to stop occurring. Do I have to do anything before running the tests for the first time? @sneridagh , any tips?

@sneridagh
Copy link
Member

Add it as a devDependency

@wesleybl
Copy link
Member Author

Add it as a devDependency

Yes this works. Should we add it to the app generator's devDependencies?

@sneridagh
Copy link
Member

@wesleybl I am so over Jest... We have to move far from it as soon as we can :(

I have a PR open to update all the deps/devDeps to the generator:

#5872

I will add it there. I plan to add all the required dependencies there for 18 (apps/plone repo folder in the Volto repo), so we have them all already in projects/addons. Then we could go to pnpm also right away.

@sneridagh
Copy link
Member

@wesleybl I will work on it in the next days, please add @plone/types (as devDep) if you can.

@wesleybl
Copy link
Member Author

@sneridagh I was having this problem with other packages too:

Test suite failed to run
    node_modules/@plone/volto/src/components/theme/SlotRenderer/SlotRenderer.tsx:1:29 - error TS7016: Could not find a declaration file for module 'react-router-dom'. '/builds/sgc/my.package-12677/my.package/node_modules/react-router-dom/index.js' implicitly has an 'any' type.
      Try `npm i --save-dev @types/react-router-dom` if it exists or add a new declaration (.d.ts) file containing `declare module 'react-router-dom';`
    1 import { useLocation } from 'react-router-dom'
Test suite failed to run
    node_modules/@plone/volto/src/helpers/Blocks/defaultBlocks.ts:1:28 - error TS7016: Could not find a declaration file for module 'uuid'. '/builds/sgc/my.package-12677/my.package/node_modules/uuid/dist/index.js' implicitly has an 'any' type.
      Try `npm i --save-dev @types/uuid` if it exists or add a new declaration (.d.ts) file containing `declare module 'uuid';`
    1 import { v4 as uuid } from 'uuid'

I will add them too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants