plone / volto

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

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

Closed wesleybl closed 1 month ago

wesleybl commented 1 month ago

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):

wesleybl commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

Add it as a devDependency

wesleybl commented 1 month ago

Add it as a devDependency

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

sneridagh commented 1 month ago

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

https://github.com/plone/volto/pull/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 commented 1 month ago

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

wesleybl commented 1 month ago

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