plone / volto

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

Volto addons story #1433

Open sneridagh opened 4 years ago

sneridagh commented 4 years ago

@tiberiuichim proposal: https://gist.github.com/tiberiuichim/eb20c6c18210c384e57028f94d9f4b18

EEA apply Volto addons config: https://github.com/plone/volto/issues/1396

@tiberiuichim "Local config" concept: https://github.com/eea/forests-frontend/blob/master/src/localconfig.js

@nzambello CLI PoC https://github.com/nzambello/voltocli

We can put on the table how we do addons in kitconcept too, using yarn workspaces.

tiberiuichim commented 4 years ago

There's one underlying difference of approach between the method I've been working on (the one with applyConfig) and Nicola's, but I think they can be combined.

Nicola's has developed a really nice voltocli utility, which can generate a consistent package infrastructure. The packages I've built were simply npm init generated.

Now the real difference comes: my method allows addons to be "fire and forget", while Nicola's recommend and document how to include the package in your setup, so that you can use it. The nice part is that he uses a pattern of passing options, which allows more flexibility. We didn't have a case where we wanted to customize how an addon works, for that I would use ~/config.settings for the simple cases and something like Nicola's method for stuff that's more complicated.

sneridagh commented 4 years ago

@tiberiuichim I can't see @nzambello pull config approach from the cli (also, if you don't provide a valid git repo with permissions on it, the create recipe fails). I couldn't tell from the code. Maybe you can point me to them! @tiberiuichim also, having a full view of your approach will be great! I only have the two pointers you gave me terms of the "extender".

I'm realizing that the scheduled metting for May 8th is too far :) and we should have a preliminary one before :)

sneridagh commented 4 years ago

So an add-on can provide:

The project should be able to catch all of them and apply them. Did I miss something?

@tiberiuichim How do you apply the routes/reducers?

Helpers:

Question, what happens if two different addons collide with some of them? Like setting the same route or provide the same reducer name? I guess the last one will win, but then the behavior of the resultant project will be erratic.

nzambello commented 4 years ago

@tiberiuichim I can't see @nzambello pull config approach from the cli (also, if you don't provide a valid git repo with permissions on it, the create recipe fails). I couldn't tell from the code. Maybe you can point me to them! @tiberiuichim also, having a full view of your approach will be great! I only have the two pointers you gave me terms of the "extender".

I'm realizing that the scheduled metting for May 8th is too far :) and we should have a preliminary one before :)

I guess @tiberiuichim was talking about this https://github.com/collective/volto-multilingual-widget#custom-widget

sneridagh commented 4 years ago

I'm fiddling with the released package feature, and now I've realised that we will have a problem if we want to maintain the src structure in the released packages. Setting up yet another "magical" Webpack alias for declared "addons" seems to me like too much.

We use the src in our addons too, and I can see that the EEA folks as well. I think I'm more biassed to use a release script that puts all the src content in the released package root. Then imports to the root will work:

import { DummyView } from 'volto-testaddon/components'; instead of import { DummyView } from 'volto-testaddon/src/components';

What do you think?

tiberiuichim commented 4 years ago

@sneridagh : On EEA projects we've tweaked the razzle.config.js to include aliases to the src of folder of each addon. See https://github.com/eea/volto-base/blob/master/src/razzle.config.js#L187

The plan was that, if addons will be transpiled and released as npm packages, we'll write main entrypoint in package.json and take things from there.

To answer your earlier comment:

For addon routes we have our own code in ~/routes.js that import { addonRoutes } from '~/config' and includes them. See https://github.com/eea/forests-frontend/blob/master/src/routes.js. We include addon routes before the default Volto routes and the router uses the route that matches first, so they work as an override of the default routes.

On the topic of configuration conflict: that's a tough question.

In our addons we've added some settings that allow customization of Volto by the addons. While I'm not a fan of customizations, I wouldn't feel comfortable shipping an addon that customizes (jbot style) a Volto file. For the limited domain of an organisation's projects, I think that's fine, there's a limited number of users for that addon and they understand the implications. Actually I think we only have one such customization (Volto is customized by an addon) and I think that can be solved in simpler way.

A system that would detect configuration conflict would be nice, but it might complicate things. Looking at Plone for inspiration, configuration conflict there is easy to detect when code base is organized around the concept of ZCA components. The possibility to detect conflicts also implies that these conflicts can also be resolved in a way, to allow "one part to win".

As a matter of API, I can think of doing something like this. Let there be a noConflict function that can wrap an addon "inclusion" configuration function and trigger an error if it detects that the addon changes a configuration setting, instead of just adding to it. So, taking the original:

import * as voltoConfig from '@plone/volto/config';
import {
  installImageSlides,
  installTableau,
  installNews,
} from 'volto-addons/config';

import { applyConfig as installFiseFrontend } from './localconfig';

const config = [
  installImageSlides,
  installTableau,
  installNews,
  installFiseFrontend,
].reduce((acc, apply) => apply(acc), voltoConfig);

This can then become:

const config = [
  noConflict(installImageSlides),
  noConflict(installTableau),
  installNews,
  installFiseFrontend,
].reduce((acc, apply) => apply(acc), voltoConfig);

So, some addons can be wrapped in this noConflict function that would detect if the addon would change the overall Volto configuration. To twist this around, this could also become like this, but the reduce call would need to be changed.

const config = [
  installImageSlides,
  installTableau,
  safeOverride(installNews),
  safeOverride(installFiseFrontend),
].reduce((acc, apply) => apply(acc), voltoConfig);

Is something like this worth doing? I don't know... in Plone configuration conflicts are worth it as they're easy to achieve: the configuration registry is immense and the whole system architecture had to be changed to enable this.

tiberiuichim commented 4 years ago

@sneridagh I think addons should also be able to provide razzle.config.js tweaks. It might be overkill, but for our projects it's easier to ship a really powerful addon and to maintain in a single place its tweaks, then it is to maintain, in the long term, on multiple projects, an always changing config.js file or razzle.config.js.

sneridagh commented 4 years ago

I created https://github.com/collective/volto-example-project-addons as documentation on how to implement the basic boilerplate for addons in a Volto project. We can continue from here. In the future, @nzambello CLI could set all this up for you.

sneridagh commented 4 years ago

@sneridagh : On EEA projects we've tweaked the razzle.config.js to include aliases to the src of folder of each addon. See https://github.com/eea/volto-base/blob/master/src/razzle.config.js#L187

That is no longer needed because: https://github.com/plone/volto/pull/1438 https://github.com/plone/volto/pull/1442

tiberiuichim commented 4 years ago

Things that might be interesting to be able to change in an addon:

Other stuff relevant to addons:

tiberiuichim commented 4 years ago

To facilitate development of addons, I've had to tweak the jsx compilation rule. This is my razzle.config.js in a clean Volto project where I've added a single addon.

const jsConfig = require('./jsconfig').compilerOptions;

const pathsConfig = jsConfig.paths;
let voltoPath = './node_modules/@plone/volto';
Object.keys(pathsConfig).forEach(pkg => {
  if (pkg === '@plone/volto') {
    voltoPath = `./${jsConfig.baseUrl}/${pathsConfig[pkg][0]}`;
  }
});

const config = require(`${voltoPath}/razzle.config`);
const razzleModify = config.modify;

module.exports = {
  plugins: config.plugins,
  modify: (config, { target, dev }, webpack) => {
    const vc = razzleModify(config, { target, dev }, webpack);
    const jsxRule = vc.module.rules.find(
        module => module.test && module.test.toString() == /\.(js|jsx|mjs)$/, // eslint-disable-line
    );
    const jsxIndex = vc.module.rules.indexOf(jsxRule);
    jsxRule.exclude = [/src\/addons\/.+\/node_modules/];
    vc.module.rules[jsxIndex] = jsxRule;
    return vc;
  },
};

I have this simple volto project where the addon is integrated and working for development: https://github.com/tiberiuichim/volto-slate-project.git

sneridagh commented 4 years ago

Why did you have to do that? I think that with the fix I pushed to the new branch all is good now with the deps hoisting problem.

tiberiuichim commented 4 years ago

I'm using Volto master, not the branch, but I don't see how the new branch would help.

My addon hasn't been specified as a dependency to the project, yet. I run "yarn install" in each addon folder. If I'd specify the addon as a disk dependency, then npm (I haven't tried with yarn) creates a copy of the addon in node_modules. Using npm link would defeat the purpose of the addon concept and I think it will run into trouble with compilation, anyway.

"postinstall": "yarn omelette; yarn develop; for f in src/addons/*; do cd $f && yarn install && cd -;done ",

If the addons/*/node_modules is not excluded from jsx compilation, there's problems because webpack treats distributed code as JS modules.

sneridagh commented 4 years ago

Try with workspaces, really, works like a charm now. Did you take a look to my demo repo?

https://github.com/collective/volto-example-project-addons

Take a look and let me know what you think. Any other solution is far more convoluted/complicated.

Also the step dev-released is almost seamless.

Cheers, V.

On Wed, 13 May 2020 at 08:03, Tiberiu Ichim notifications@github.com wrote:

I'm using Volto master, not the branch, but I don't see how the new branch would help.

My addon hasn't been specified as a dependency to the project, yet. I run "yarn install" in each addon folder. If I'd specify the addon as a disk dependency, then npm (I haven't tried with yarn) creates a copy of the addon in node_modules. Using npm link would defeat the purpose of the addon concept and I think it will run into trouble with compilation, anyway.

"postinstall": "yarn omelette; yarn develop; for f in src/addons/*; do cd $f && yarn install && cd -;done ",

If the addons/*/node_modules is not excluded from jsx compilation, there's problems because webpack treats distributed code as JS modules.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/plone/volto/issues/1433#issuecomment-627765377, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADW4D6X2VG6SQEUMB3FXMLRRIZ27ANCNFSM4MSD52AQ .

-- Víctor Fernández de Alba Github/Twitter/IRC: sneridagh