plone / volto

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

Hoisting problems on @babel dependencies in projects #1897

Closed sneridagh closed 3 years ago

sneridagh commented 4 years ago

Problem: There's a weird thing that happen if this requisites are met:

if these conditions happens, yarn decides to hoist (not use the main node_modules to place (some, not all which is weird) @babel plugins, and places (hoist) them in @plone/volto/node_modules, provoking the build to fail, since @babel expects to have all its plugins in the top level node_modules.

Solution: Keep in sync the Volto @babel/core version with the one in babel-preset-razzle.

tiberiuichim commented 4 years ago

@sneridagh I've just had a pretty nasty encounter with this problem.

As well for future reference, here's my story:

I've tried to make a Volto branch that merges 2 PRs: multi block copy and the 404 route. The multiblock copy introduces a new dependency, the localstorage stuff, so it changes yarn.lock. So after a bit of fiddling around, I've realized that one important thing is to make sure both branches are up to date with Volto master. So I've updated the multiblock copy branch and generated a new yarn.lock file with it. The problem is that this yarn.lock upgrades the @babel stuff, but this doesn't appear to be a problem for the branch itself, Volto starts with no problem.

Now in the Volto project I've added my custom Volto release as a dependency and Volto crashes with hoisting problem. Then I've tried a lot of things, among them: bringing in yarn.lock from the Volto release and running yarn (in the project) on top of that, removing node_modules, etc. In the end I've even downgraded to my "stable" Volto release, but that didn't help either. Which is weird, right?

The solution, for some reason, to make it work with my downgraded Volto was to run yarn cache clean. That finally got Volto running. Now I'll need to see how to upgrade all of this.

sneridagh commented 4 years ago

@tiberiuichim for curiosity, I tried npm@7... did you try it? I think I have a suspicions on why is all this happening...

It has to do with our Python background and the desire to pin-them-all dependencies, while the JS world takes just another approach and we have to learn to think using the hoisting based dependency resolve algorithms...

A lot to talk about this...

tiberiuichim commented 4 years ago

@sneridagh My impression is that it's due to the duality of Volto as a standalone app and Volto as an app-building framework. We develop and test on Volto standalone, but that's not realistic to the way we deploy, as Volto projects. I think this can be overcome with better dependency declarations and tooling. For example, our Volto projects are "razzle projects", I would want to see a direct dependency on razzle and not indirect, provided by Volto.

tiberiuichim commented 4 years ago

@sneridagh you know we use to develop addons and Volto projects with npm (I think npm 5?) without workspace support from yarn, and that meant no hoisting at all. It was ok, just needed more tooling, to run "npm install" in each addon.

I haven't really fixed my problem from the other day, I wasn't able to beat that hoisting issue.

I would define the problem like this:

So to me the obvious solution is: make the Volto project have its own dependencies properly specified, so that we don't care if there's hoisting shenanigans.

sneridagh commented 4 years ago

then you'll have to specify devDependencies (all babel and eslint and friends) in the project, right?

sneridagh commented 4 years ago

My Python me is telling me to make a package defining those, but we will fall into the same caveat!

tiberiuichim commented 4 years ago

Yes, I think that's the proper way. I've tried adding razzle (or razzle-preset? can't remember) as dependency, but I there was some other stuff missing

sneridagh commented 4 years ago

So, then we will need more than ever a good project generator and config updater in place.

tiberiuichim commented 4 years ago

@sneridagh we should do a volto-babel-preset

tiberiuichim commented 4 years ago

But I'm surprised though, in theory all we need is razzle preset, I think. I'll have to dig into this tomorrow and understand the problem anyway, I need to deploy those changes in the end

tiberiuichim commented 4 years ago

Another experience with the hoisting problems. I'm not running anything weird.

My setup:

I have a basic yo generator based Volto project. Nothing changed except to add a new test addon that I'm working on.

Steps to reproduce:

I add a new dependency to the addon by running

yarn workspace @plone/datatable-tutorial add papaparse

When I try to start with yarn start I have the hoisting problem:

Module build failed (from ./node_modules/babel-loader/lib/index.js): 
Error: Cannot find module '@babel/plugin-proposal-function-bind' from '/home/tibi/work/myvoltoproject'

Now I just run yarn, nothing else, and it clears the problem. I've hit this issue two times, whenever I've added dependencies to my addon, so it's repeatable.

sneridagh commented 3 years ago

Solved in Volto 9.