-
-
Notifications
You must be signed in to change notification settings - Fork 10
Update frontend_addon template to support Volto 19 #292
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
Conversation
|
@davisagli Can't we have |
See: #259 |
@wesleybl I'm not interested in supporting two different test frameworks for a single major version of Volto. If there are multiple ways to do things, it's harder to document and harder to tell new folks what to do. If you really need it and know what you're doing, you can already (with this PR) build a frontend addon for Volto 18 using vitest, like this: |
|
@davisagli @wesleybl I did a bit of research and asked the machine too, and I got this answer:
I don't like to have to micromanage this, but I see the point. However, I'd add these at least to the list: core-js / core-js-pure / es5-ext (polyfills and shims, the build might be using them) I will update the PR if you all agree. |
|
@sneridagh Of course. If the scripts for these packages are necessary, we can add them. By default, pnpm 10 doesn't run the scripts unless explicitly specified. I tried to keep the default. Since the tests passed, this was an indication that we didn't need them. But if we do, we can add them. What I don't think we should do is add them unnecessarily. |
|
@davisagli Why didn't you want to keep |
|
@davisagli @sneridagh Another thing we need to do is add support for Node.js 24 in Volto 19. I tried running the Volto tests from cookieplone with Node 24: https://github.com/plone/volto/blob/main/.github/workflows/cookieplone.yml#L16 but I got an error that the Node version wasn't supported. I think the https://github.com/search?q=repo%3Aplone%2Fcookieplone-templates%20SUPPORTED_NODE_VERSIONS&type=code |
|
Another thing is that we will have to have a test matrix for Volto 18 and another for Volto 19. Maybe the |
templates/add-ons/frontend/{{ cookiecutter.__folder_name }}/Makefile
Outdated
Show resolved
Hide resolved
templates/add-ons/frontend/{{ cookiecutter.__folder_name }}/Makefile
Outdated
Show resolved
Hide resolved
templates/add-ons/frontend/{{ cookiecutter.__folder_name }}/package.json
Show resolved
Hide resolved
It could be useful information if the developer of a Volto project wants to use those packages directly, even if Volto itself does not.
Good point, I added it to the to-do list in the PR description.
Yeah, that would be ideal. |
I find these warnings strange. It seems like something is wrong. But that's okay with me.
Would it also be necessary to add a task to update Volto tests with version 24? Or will you remember that? |
Let's check what the lifecycle scripts are actually doing...
These just have a postinstall script which prints an ad banner (https://github.com/zloirock/core-js/blob/master/packages/core-js/postinstall.js).
This one has a postinstall script which prints a call for peace when installed in a Russian timezone (https://github.com/medikoo/es5-ext/blob/main/_postinstall.js).
This one looks necessary, to build a Go binary.
This has a postbuild script but nothing on install.
This has a postinstall script which is the entire point of the package (to download locales).
This has a postinstall script which looks necessary.
This has a postinstall script which just prints a line about how to use it. Actually, do we still need this? pnpm has its own |
@sneridagh where can we find an example of these changes? |
@davisagli I would try using Cypress 15 on the Volto 19. But we would have to do it on the Volto first. And I believe it would be transparent here. |
|
@davisagli I will look into Storybook ones. I already did for Seven. |
|
@davisagli @wesleybl I added plone/volto#7562. I can think in a couple of use cases where it will come in handy. It needs this one to be green though. There's also: plone/volto#7538 When it's merged, I think we can go with this one. Also: after spending some hours in trying to update Storybook to 9, I abandoned. It's few things that it would give us in exchange for the lots of effort that it has to be done to make it work. Reason? They are focusing in a very sleek Vite build, so they slimmed down all deps and build deps to the minimum, so if you really need them, you have to provide them yourself... anyways, I think it's not worth the effort, and the things that Storybook 8 gives you are lots and we underuse them anyways... |
|
@sneridagh I think after adding a test matrix for the alpha version, we can merge this one here. |
|
We also need to add the packages to |
|
I think now it's ready. @davisagli @wesleybl take a final look and let's make it happen. The tests are failing because https://json.schemastore.org/hatch.json is down... |
|
|
@sneridagh, @davisagli, @wesleybl: There's a 1.0,0 release of pytest-jsonschema. |
|
@davisagli @wesleybl I'd say let's merge it, right? or there is something else left? |
davisagli
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. (Sorry, I'm hosting family this weekend and not paying much attention.)
| run: | | ||
| make ci-acceptance-test | ||
| frontend-functional-prerelease: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sneridagh I was thinking of creating an matrix in the existing job, instead of creating a new one. That way we'd avoid code duplication. Maybe I can try that later.
Fixes #206 ... or at least a start
So far I only added vitest, based on #207.
First I started trying to add subtemplates for volto 18 and volto 19 which could override specific files. But that quickly became a multiplicative problem because we needed different Makefiles and package.json for volto 18 and volto 19, but also different Makefiles for the standalone frontend_addon template vs when it's used in a project. So I abandoned that approach and switched to more targeted conditionals instead.
To do:
If you want my help, I need an example where you can point out all the specific changes that are needed.