Patternfly-react: Split build process

[karelhala]

TL;DR - let's improve our script commands in packages!

With improved folder structure (which is really great!) we should somehow split build process for PF3, PF4 and other packages.

Since we are using monorepo, yarn and lerna this might be easier than we think.

Proposal

One thing to note is that we could treat folder pf3 and pf4 as npm packages so we could put package.json in each of them and add our custom script tasks in there.

packages/
  eslint-plugin-patternfly-react/
  react-codemods/
  patternfly3/
    patternfly-react/
    react-console/
    package.json - set to private
    README.md
  patternfly4/
    react-core/
    react-docs/
    ...
    package.json - set to private
    README.md

I know we have quite complex build process right now, but we could identify which process is good for which package and move them into pf3 or pf4. If any task should be present for both folders well we'll have to put them in both of them.

List of scripts

• bootstrap - needs to stay at root package.json
• build - needs to stay, but needs to be changed
  
  
  
  • perhaps lerna run build and each package.json will have it's own build, if not then you should write one
  
  
• build:* - specific for pf3 or pf4
• clean - nice to have in root
• commit - needs to stay in root
• coveralls - needs to stay in root
• generate - needs to stay in root
• lerna-semantic-release - needs to stay in root
• lerna - needs to stay in root
• lint - should be moved to pf3 and pf 4
• lint:* - should be moved to pf3 and pf 4
• precommit - not sure about this one, but I guess lerna run should take care of this and if package is interested in precommit phase it should be there
• prettier - should be moved to pf3 and pf4
• semantic-release - needs to stay in root
• start - should be rewritter to lerna run start --concurently and each package should have it's own run command + it's possible to pass yarn start --scope=some-package
• start:docs - great example of start in folder pf4
• storybook - great example of start in folder pf3
• storybook:* - should be moved only into pf3, not sure how to exactly run it from root rather then yarn lerna run storybook:* --scope=patternfly-react from command line, but that could be pointed out in docs and if you want to run this from pf3 folder you'd have to run yarn storybook:*
• stylelint - should be moved to pf3 and pf 4
• test - needs to stay in root and has to be rewritten to lerna run test + every package has to have it's test command, this will be the hardest part probably (mostly to share one config, but be possible to run tests for one package)
• test:* - should be part of pf3 and pf4
• travis-deploy-once - needs to stay in root

So given the list above there would be 13 scripts in root folder instead of 35 and we'd have possibility to run any command in any package by calling yarn lerna $COMMAND --scope=package_name.

Improvements

1) Asynchronous build on travis
2) Faster build on local machine
3) Custom builds per each package
4) Cleaner packages

[karelhala]

Honestly I don't know who should take care of this one @jschuler @priley86 @jeff-phillips-18 if you want I can help you with setting it up. I've already did POC on monorepo last time and this should be somewhat similiar to what I done.

[priley86]

@karelhala - i think this proposal sounds nice and would give some more granularity to the monorepo build we have today. I _think_ I agree on almost all of these assessments, but maybe a little clarity on some would be nice:

• prettier: do we intend to have separate configurations? Today they are same globally (in pf3 react/pf4 react/ and pf next). I'm uncertain if we need to target other configurations for pf4 react though.
• stylelint: the same would apply - you'd think we'd want the same settings, but it seems like you'd prefer the flexibility to adjust these?
• lint (and eslint): i agree w/ this assessment. It would be nice to move pf4 react to tslint / tslint configs if we ever hope to enforce more strict types. Ideally we could expose those to downstreams like we do w/ eslint-plugin-patternfly-react today. One could argue that eslint-plugin-patternfly-react is more closely associated w/ patternfly3, but it's just a set of eslint configurations so maybe we keep tslint/eslint configs at the package level?

@karelhala and yes, if you would like to prototype this build, by all means. It would also be wonderful to look at lerna-changelog or some utility to create CHANGELOG.md with this effort. @dgutride noticed this in Babel and mentioned to us, I like this suggestion a lot. Ex changelog:
https://github.com/babel/babel/blob/master/CHANGELOG.md

[LHinson]

@priley86 @karelhala Just wanted to give you a heads up that we have a couple PF4-React components (nav and page layout) ahead of this issue in priority, however @jschuler will be picking this up after those are complete.

[karelhala]

@LHinson awesome! This should speed up the build process a lot.

@priley86 great points! I wanted to raise this issue so it wouldn't be forgotten and to have at least something to refer to.

[priley86]

@jschuler @karelhala - i have started this effort in #625.

Regarding:

"One thing to note is that we could treat folder pf3 and pf4 as npm packages so we could put package.json in each of them and add our custom script tasks in there."

I think this is possible, but prefer to keep builds package specific. Maybe the private packages under pf3/pf4 are better suited for lint settings/lint scripts and devDependences? We can soon move many of those from the root after. Agree that storybook belongs there as well.

Also - I noted that we currently cannot run @patternfly/react-core and @patternfly/react-styles builds in parallel (since react-core depends on react-styles). All other builds are parallel safe. Any suggestion?

Will continue next week...

[priley86]

so after thinking this a bit further, I do tend to see why the pf3/pf4 private packages may be desirable, but would like to raise some questions first. We could run lint tasks from the private packages for each respective sub pf3/pf4 package, however I think this raises the following questions:

• eslint - we currently have the same eslint rules globally for all JS (this includes PF3 React and all PF4 react es6 source). Originally this was preferred since the plan was to continue developing es6 in PF4 and expose Typescript types via type definitions. It appears this is no longer preferred and we'd like to develop strictly in Typescript for PF4. If that is the case, then eslint is potentially no longer necessary for PF4 packages and should not be configured globally, but would just like to confirm this before we do this.
  
  
  
  • should es6 source currently in PF4 / react components be removed alongside this PR or in a future PR? My assumption is that we would continue to keep eslint in place for now, just to run it separately (as separate lint scripts) in the scope of this PR.
  
  
  • should we ignore linting on eslint-plugin-patternfly-react, react-codemods, and react-icons, or potentially look at moving them within the pf3/pf4 directories (linting would no longer apply globally to them)?
  
  

[priley86]

@dgutride @seanforyou23 @dlabrecq @jeff-phillips-18 any additional thoughts here?

[karelhala]

eslint tasks will probably be the hardest thing, I think that your suggestion is probably the best, running multiple times eslint once for PF3 and second time for PF4. Once we finalize on TS rules and such we should dive into it and introduce tslint for PF4 components.

About the second point

should we ignore linting on eslint-plugin-patternfly-react, react-codemods, and react-icons,

I have no strong feelings one way or another.

[dlabrecq]

Not only would I like to see the build split between PF3 and PF4, but react-core as it's own build. That way, we can easily add typescript to react-core without globally affecting PF3, eslint, etc. We could even use tslint with just react-core if desired.

[ohadlevy]

@dlabrecq I wonder if its desirable to have typescript in pf4? would you add it to pf3 as well? I'm mostly asking (not because i care either way) but as yet another thing that makes transition from pf3 -> pf4 harder.

I for one, would like to see project standards align vs divorce, I really dislike the disconnect between pf3 and pf4. what would be the technical reasons to have a different standards across packages?

[dlabrecq]

I'm not proposing adding typescript to PF3, only PF4.

[dlabrecq]

Note that just because PF4 is built with Typescript, it doesn't mean you're app needs to use it. We already provide Typescript types via type definitions.

The problem with type definitions is they're created manually, which is prone to error. You don't necessarily see an error until you consume the component. If PF4 was built with Typescript, the type definitions would be generated for us.

[priley86]

so this is a little more involved than I originally realized, but still completely possible should we decide to continue down this route...

I've pushed some more updates in #625 and updated status. I am going to have to shelf this work and continue iterating as I have time.