PulseTile / PulseTile-RA

PulseTile exploration with React Admin
Apache License 2.0
5 stars 6 forks source link

migrating PulseTile to Lerna based approach in line with RA #49

Open tony-shannon opened 5 years ago

tony-shannon commented 5 years ago

need to be able to accommodate Helm NES Scotland Showcase from PT RA base version

tony-shannon commented 5 years ago

@BogdanScherban to update here about his ideas to use https://github.com/lerna/lerna for this challenge

BogdanScherban commented 5 years ago

@tony-shannon @fzaninotto I want to describe my understanding of this issue.

We should create new project via Lerna. The project will have following structure:

|
|-- packages/
|---- core/
|---- plugins/
|------ feeds/
|------ topThreeThings/
|------ vaccinations/
|---- features
|------ userTour/
|------ contractMode/
|
|-- versions/
|---- helm/
|------ topbar/
|------ footer/
|------ index.js
|---- scotland/
|---- showcase/
|
|-- package.json
|-- lerna.json

So, we should create set of packages for Core, plugins, features etc. These packages should be imported in version files, for example:

// helm/index.js
import React, { Component } from "react";

import Core from "core";
import Feeds from "plugin-feeds";
import Vaccinations from "plugin-vaccinations";
import TopThreeThings from "plugin-topThreeThings";

const plugins = [
    { name: "Feeds", component: Feeds },
    { name: "Vaccinations", component: Vaccinations },
    { name: "TopThreeThings", component: TopThreeThings },
];

class Helm extends Component {
   render() {
       return(
            <Helm
               core={Core} 
               plugins={plugins} 
             />
       );
   }

}

So, I should find a way, how to do following: 1) How to create new packages from Core, plugins etc. 2) How to import Core/plugins as package; 3) How to create a build for each version.

fzaninotto commented 5 years ago

One question: why do you split the base code into so many packages? Why not a single core package? I don't understand the rationale for splitting plugins and features from the core.

BogdanScherban commented 5 years ago

@fzaninotto I want to do this because different versions will have different set of plugins:

helm = Core + (Vaccinations + TopThreeThings + Feeds) scotland = Core + ReSPECT (plugin, will be done in future) showcase = Core + all possible plugins etc.

fzaninotto commented 5 years ago

You can achieve this even if the plugins live in the same package as the core. The difference will be, e.g. in Helm:

// with one package for the core and one package per plugin
import { List } from 'pulsetile-core';
import { Vaccination } from 'pulsetile-plugin-vaccination';
import { TopThreeThings } from 'pulsetile-plugin-topthreethings';
import { Feeds } from 'pulsetile-plugin-feeds';

// with one package for the core and plugins
import { List, Vaccination, TopThreeThings, Feeds } from 'pulsetile-core';

I think it's better for app developers, and I don't see the benefit for you of having plugins listed as different packages.

fzaninotto commented 5 years ago

Sorry, wrong button, I closed the issue by mistake.

BogdanScherban commented 5 years ago

@fzaninotto Francois, I tried to create project via Lerna, but probably, I don't quite understand how Lerna works. Could you please tell me, what I did wrong?

I did following: 1) Create new project by lerna init 2) Added new package pulsetile-core 3) Installed all required dependencies by lerna bootstrap 4) Created new project by npm create-react-app helm 5) Try to import pulsetile-core package by:

import Core from "pulsetile-core";

...and take an error that "pulsetile-core" can't be found

I have an assumption, that I should firstly: 1) publish "pulsetile-core" as node-module at npmjs.com 2) add it to package.json 3) install it, and after these it will be available.

I looked at react-admin package.json - it includes all required RA-packages: https://github.com/marmelab/react-admin/blob/master/examples/simple/package.json

But if I understand correctly, there aren't links to packages/ directory. There are rules for npm install, aren't them?

My results was saved here (my fork, but only for experiments): https://github.com/BogdanScherban/PulseTile-Lerna

Could you please tell me, where I wrong?

fzaninotto commented 5 years ago

Create-react-app isn't yet perfectly compatible with lerna, and that may be the cause of your issue. Here is how we usually do:

Preferably use yarn over npm as I'm not sure that npm handles monorepos correctly.

At this stage, normally the helm app should load the pulsetile-core package. If it doesn't, I'll checkout your fork and try to make it work.

BogdanScherban commented 5 years ago

@fzaninotto Francois, I tried to solve the issue by this way: 1) removed helm 2) created helm by cd versions && create-react-app helm 3) installed dependencies globally by yarn (cd ../ && yarn)

Result is the same - module 'pulsetile-core' is unavailable. I saved results in my fork: https://github.com/BogdanScherban/PulseTile-Lerna

Could you please checkout it if you will have free time?

fzaninotto commented 5 years ago

In the lerna.json, try adding the versions/* as packages, so that lerna knows about your helm project

{
  "packages": [
    "packages/*",
+   "versions/*",
  ],
  "version": "0.0.0"
}
BogdanScherban commented 5 years ago

@fzaninotto

Thank, Francois - it helps! I moved helm to packages, and pulsetile-core can be found here.

Right now I try to render Core:

1) I run npm start in helm; 2) I have a result: 111111111111

I tried few variants:

import Core from "pulsetile-core";
export default class App extends React.Component {
    render() {
        return (
            <Core />
        );
    }
}

or

import Core from "pulsetile-core";
export default Core;

I'm trying to find a solution.

fzaninotto commented 5 years ago

Why did you move helm to packages? If you modify the lerna.json as I explained, it's not necessary.

The bug is due to ES6 not being transpiled by babel in the core. You probably need to build (=transpile) the core code first, then run the helm project.

BogdanScherban commented 5 years ago

@fzaninotto Francois, I moved versions/helm back to root - yes, it was useless. Pulsetile-core is available.

But how can I transpile core? In https://github.com/marmelab/react-admin/tree/master/packages each package use:

   "main": "lib/index.js"

Unfortunately, this file in gitignore. But what should be here?

Besides this, I tried to create build by npm run build and use it, but without results too.

fzaninotto commented 5 years ago

In react-admin, each package is transpiled by TypeScript (Babel in your case) from ES6 to ES5. See the scripts section of the package.json of each package for details. You need to set up a build task for the core, too. If you don't know how to do that, read the babel.js documentation.

No need to do that in HELM - Create React App does it on its own.

BogdanScherban commented 5 years ago

@fzaninotto

Francois, it works!!!!! I saved the latest version: https://github.com/BogdanScherban/PulseTile-Lerna

I take react-admin as example and added also two commands, to copy images directories for core and for version parts.

Thank you for your help!!!

fzaninotto commented 5 years ago

One thing I don't understand is the remaining "versions" in "core". What is it for?

BogdanScherban commented 5 years ago

@fzaninotto

One thing I don't understand is the remaining "versions" in "core".

Francois, do you mean this "version" directory? https://github.com/BogdanScherban/PulseTile-Lerna/tree/master/packages/pulsetile-core/src

At this stage it was necessary to understand, how Lerna works generally. For this reason I used current version of our project, which consists of two parts - core/ and version/ (for non-core plugins, features, config files etc.)

In future version/ directory should be removed, and each plugin should be done as independent component inside pulsetile-core package. I understand required structure like this:

import { Core, Vaccination, TopThreeThings, Feeds } from 'pulsetile-core';
import { Topbar }  from 'topbar';
import { Footer }  from 'footer';

export default class Helm extends Component {
    render() {
        const nonCorePlugins = [
               { name: "feeds", component: Feeds },
               ...
        ];
        return(
             <Core plugins={nonCorePlugins} topbar={Topbar} footer={Footer} />
        );
    } 
}

Francois, could you please tell me: 1) do you mean this "versions" in your question? 2) as I understand correctly solution of "three versions inside one repo"?

Could you please approve my idea or said, where I wrong and how should we organize version creating?

tony-shannon commented 5 years ago

Francois, do you mean this "version" directory? https://github.com/BogdanScherban/PulseTile-Lerna/tree/master/packages/pulsetile-core/src

I'm sure thats what Francois meant, it shouldnt be there, but I understand it was a temporary solution for you

fzaninotto commented 5 years ago

I understand that "versions" is only there temporarily and that you'll eventually remove it, I'm OK with that.

kbeloborodko commented 5 years ago

Hi, @tony-shannon, @BogdanScherban, @fzaninotto I have circled through Lerna's documentation and examples a couple of times and I am not sure considering the benefit of its usage for PulseTile project. If there is going to be only 1 core package like @fzaninotto suggested and Helm, NES Scotland and Showcase would be just a combination of plugins imported from core than there would be nothing that lerna should manage. Am I missing something? We can just create Helm, NES and Showcase components that will import the necessary parts of pulsetile-core and that's it.

tony-shannon commented 5 years ago

thanks @kbeloborodko We need to be clear on naming and language here

The core, -- key core reusable components - used in every project The plugins-- additional components- used in some projects -- known as packages in React Admin

Projects, which combine core + plugins + modify (extend/constrain) these components.. .. which I understand in React Admin (lerna based ) library are handled in /examples

So my understanding is that Lerna can handle core components + plugins/packages + examples/projects

NB this may not be ideal and we should debate the pros/cons, argument for external repos + generators for project purposes at this time

We may need another call to debate/discuss/decide this one

fzaninotto commented 5 years ago

@kbeloborodko You're right, if all the code for the core and the plugins live inside one single package, we need monorepos (to help manage several package.json, and yarn does that well) but not lerna (which only serves to synchronize the publication of several packages on npm).

Let me reformulate : yarn allows to have several sub packages (each with its own package.json) inside a single repo. So we need that monorepo architecture to be able to have one package.json for the core, one for the Helm showcase, and one for the NES showcase.

That being said, Bogdan's work so far is still useful as he built a monorepo.

Now about the plugins, the only interests of having them in separate packages are to:

  1. let them follow an independent release schedule (i.e. release a plugin v3.2 when core is at v6.4)
  2. reduce the package size upon installation on the development servers (but not in the production bundle, as the plugins won't be bundled if they are not used)
  3. Let developers replace parts of the code with their own

React-admin chose plugins for reason #3. If one of these reasons is relevant for PulseTile, too, then you should publish one package for each plugin.

But, you should know that having the code split across several packages is not in the interest of the users of a library - it's mostly beneficial to the developers of the library. Importing components from several packages is cumbersome, as I wrote earlier:

// with one package for the core and one package per plugin
import { List } from 'pulsetile-core';
import { Vaccination } from 'pulsetile-plugin-vaccination';
import { TopThreeThings } from 'pulsetile-plugin-topthreethings';
import { Feeds } from 'pulsetile-plugin-feeds';

// with one package for the core and plugins
import { List, Vaccination, TopThreeThings, Feeds } from 'pulsetile-core';

That's why, in react-admin, even though we split the code across several packages, users of the library only have to import 'react-admin', which groups the sub packages together.

So in the end, whether plugins live in their own package or not is mostly a business decision, to be able to market the plugins on their own, or to encourage the community to provide alternatives.

Does it make sense?

PhilBarrett commented 5 years ago

@kbeloborodko could you take a look and come back to Francois asap please

kbeloborodko commented 5 years ago

Hi, @fzaninotto, yes it does make sense.

From my point of view, it is indeed easier if we had an opportunity to import the necessary components, core or non-core from one place opposed to a workflow when we need to grab them from different places (packages).

However, from the business standpoint, we were separating those for a while and there were different reasons behind this decision: starting from the level of support and responsibility for the code (core functionality is developed by PulseTile team while plugins should be cultivated by the community) and ending with complexity and potential frequency of use (such headings as topthreethings or feeds can be specific for a particular user).

Given this background we could utilize the monorepo principle in the following way: 1) Keep core as a separate package and populate one package per plugin

This can be achieved using Yarn workspaces as we discussed before and if we need additional flexibility that Lerna provides (synchronizing release pulse) we can extend the boilerplate later and just specify yarn as a package manager (npmClient) for Lerna.

I am not sure, though, if this approach aligns with the main PulstTile concept. Initially, the core components and the plugins were put into different repositories, and this provided a decent separation of concerns. The core is maintained by PulseTile team, the plugins - by the community. Now, as it all is going to be part of one codebase, this is going to get a bit mixed up. Moreover, the projects (helm, scotland, showcase) also had their own site serving as examples of PulseTile usage, while the new concept suggests their co-placement with the other pieces of code. @tony-shannon and @PhilBarrett are the custodians of the product vision, so I am looking forward to their opinion on this matter.

CC: @BogdanScherban

tony-shannon commented 5 years ago

thanks both @fzaninotto and @kbeloborodko for your considered views on this.. a good and important discussion for us..

In summary its seems you both agree and share the view that a monorepo could have some advantages for us, esp in terms of core + packages for several versions/projects.

Kirill makes an important point re the balance of central control of the repo v contributions from the community for plugins, which we have yet to see in any volume, due to either the maturity/immaturity of the the OS/healthcare domain &/or the complexity of the challenge in developing plugins.

Let me put a related Q back to @fzaninotto here, what advice/experience do you have re handling community contributions/packages and how you handle wrt the main react-admin mono repo?

This is an important discussion for us and reckon it warrants a joint discussion between us and an opportunity to connect you both virtually face2face.

Thanks again both Tony

fzaninotto commented 5 years ago

With react-admin, we draw a clear line separating what we maintain (the core and a few plugins) and what we delegate to the community. In general, we maintain what we built ourselves and use on live applications. Our repository contains only what we maintain. As for packages maintained by the community, we only link to them.

Community contributions that can't be published as a third-party package (e.g. because there are no hooks in the core to extend) are often debated internally as to whether we should accept them or not. Most of the time, we don't. When we do, we require a high level of testing and documentation. Ideally, we publish an RFC ahead of merge to make sure that the new community feature makes sense for most users.

And of course, we accept bug fixes from the community on the code we maintain.

My experience is that community contributions are often unmaintained, unless there is a corporate sponsor who is also using the plugin. As the core package evolves and inevitably breaks backwards compatibility, community contributions become incompatible with the latest core releases. That translates into community packages that have a lower level of quality, and a lower lifespan. We could increase that quality and lifespan by integrating community contributions to our scope, but that would increase our maintenance costs - which we don't want, for the moment.

I've learned one important thing during my past 10 years of open-source contribution: one of the keys to bring life to an open-source library is frequent releases, over several years. Once third-party developers understand that you're serious building something durable, they start helping out.

I've met a few software editors who manage to get great quality contributions. They usually offer a module market (e.g. wordpress themes, magento modules) with the opportunity for third party developers to make a bit of money. That allows great contributions to emerge, with a constant level of quality.

We are currently thinking about promoting such a market for react-admin.

tony-shannon commented 5 years ago

renamed this issue from set up for 3 PT projects versions for PT-RA to migrating PulseTile to Lerna based approach in line with RA

tony-shannon commented 5 years ago

once showcase updated Bogdan will return to this task

BogdanScherban commented 5 years ago

@tony-shannon @PhilBarrett

I have a new idea, how to use Lerna for this issue.

Now Showcase is a version with maximal functionality. Suggest to use it as a base.

We will have only one package - with maximal possible functionality. Different versions of the application (Helm, Nes, London etc.) will disable functionality, which isn't used there.

|
|-- packages/
|---- pulsetile-ra/
|
|-- versions/
|---- helm/
|------ topbar/
|------ footer/
|------ index.js
|---- scotland/
|---- showcase/
|---- london/
|
|-- package.json
|-- lerna.json

Parameters, which we can pass to pulsetile-ra: 1) Topbar; 2) Footer; 3) Plugins list (only list, all possible plugins are located in pulsetile-ra) 4) Pages list (only list, all possible pages are located in pulsetile-ra) 5) Sidebar menu structure.

This approach gives possibility don't create a lot of packages for each plugin and page.

Can I try to realize this approach?

kbeloborodko commented 5 years ago

Hi, @BogdanScherban, thank you for the write up. I hope you can clarify a couple of questions considering the approach you are suggesting: 1) Lerna is used to manage versions and dependencies across different packages. If we have one package that contains the complete functionality and configure the versions by subtracting the modules what will be Lerna's role in this case as pulsetile-ra will be the only dependency? 2) How will the PulseTile plugins be exposed to external contribution? 3) Should we manage versions of the PulseTile instances (helm, scotland, showcase, london)? If yes, how would we do that? CC: @tony-shannon @PhilBarrett

tony-shannon commented 5 years ago

Hi @BogdanScherban this needs an update in response to Kirills @kbeloborodko Q and discussion with @fzaninotto

BogdanScherban commented 5 years ago

@kbeloborodko @tony-shannon

Hi, Kirill! I see following solution.

All possible plugins, pages and features are presented in package.

But we have few places in the project, where we can disable them:

1) Plugins - it is enough to disable required plugin in this list: https://github.com/PulseTile/PulseTile-RA/blob/showcase/src/version/config/nonCorePlugins.js

2) The same - for pages: https://github.com/PulseTile/PulseTile-RA/blob/showcase/src/version/routes.js

3) And for sidebar menu and different features: https://github.com/PulseTile/PulseTile-RA/blob/showcase/src/version/config/theme.config.js

Probably, it is necessary to make some changes in the project, and after this we will only disable functionality, which shouldn't be in the version.

tony-shannon commented 5 years ago

NB See related discussion here in Bogdans repo

https://github.com/BogdanScherban/Lerna-PulseTile/issues/1

BogdanScherban commented 5 years ago

Current situation for versions:

Showcase - done and upload to https://showcase-dev.ripple.foundation

Helm - done, but can't be upload because of blob-settings on https://helm-demo.ripple.foundation

Nodered - done and upload to http://nodered-showcase.ripple.foundation

NES - done, but didn't upload;

OneLondon - in process (we should add possibility to refuse create/update all headings except TopThreeThings)