Patternfly-react: PF4 dev-env start script hangs

[seanforyou23]

Hello!

I think there is an issue with the startup script for pf4 react. When I run yarn start:pf3, a development environment is spun up without problem, when I try the same thing for pf4 with yarn start:pf4, I see failed references to @patternfly/react-tokens and @patternfly/react-icons.

https://gist.github.com/seanforyou23/7943a776da224de72368859cb71d13fd

I'm running

• macOS 10.13.6
• yarn 1.9.4
• node 8.11.4
• npm 5.6.0

I get the same effect on another machine I have running yarn 1.7.0 and node 8.11.2. Speaking with @priley86 seems like node 8.9.4 and yarn 1.7.0 is a good combination. This is as much I know of the issue at the moment, working on setting up another machine now, will give it a test there also and report findings.

[priley86]

I think it would be nice to document this and support as many Node 8+ configurations as possible (since that is the current LTS release). Thoughts? Thanks for reporting 🙏

[seanforyou23]

Agree, documenting this definitely would be nice. I mean, I guess that starts here? https://github.com/patternfly/patternfly-react/blob/master/package.json#L11

And can be improved to deserve a mention in the main readme, and could be improved further as a prestart:pf4 script to give developers warning if "node" version is set to something known to be problematic - all as needed.

Maybe for now we can just start by making the package.json reflect what's accurate. So far I see 8.11.{2,4} is no good on my end.

I guess I should ask, are we confident it's the node version that's the problem here? It's a new build system for me so not sure what all we may be up against.

[mcoker]

fwiw, I get the same.

macOS 10.13.6
yarn 1.10.1
node 8.12.0
npm 6.4.1

[michaelkro]

For some reason, things are working on my personal machine

macOS 10.14
yarn 1.9.4
node 8.12.0, 8.11.3, 8.11.4 (tested different versions using NVM)
npm 6.4.1

[guillaumevincent]

Same issue on Fedora 28
yarn 1.10.1
node v8.11.2
npm 6.4.1

[dmiller9911]

So if react icons and react-tokens have not been built this will happen. I don't see those being built anywhere before the docs start. Have tokens and icons been built for anyone?

[michaelkro]

I always run yarn build manually every time I pull latest master

[guillaumevincent]

@dmiller9911 is right
need to run yarn build before running yarn state:pf4

It needs to be documented somewhere

Thank you Doug

[dmiller9911]

To make the build "shorter" the minimum is lerna run build --scope=@patternfly/react-tokens,@patternfly/react-icons It would be cool to write something to generate the scope though based on dependencies of the project or potentially allow the project to set it in package.json somewhere. This is an issue we have hit in the past, and it would be nice to find a good solution to it.

[priley86]

yes - we can go down the route of targeting specific package builds like @dmiller9911 suggests, but I refrained from this initially for fear adding too many specific build steps in the start script (or having to maintain those separately) and I guess my preference would be not having to rebuild each time when running yarn start:pf4 to restart Gatsby...

for that I would probably just add targeted scripts that are separate...

for the original issue, I would probably just keep this as simple as possible and just do a full build on postinstall (root package.json):
"postinstall": "yarn build"

thoughts?

[dmiller9911]

As a heads up.postinstall will run for anyone consuming the lib. There is a prepare script that only runs on local yarn install, but it also runs before publish so it could run more than once in a CI install.

I think I would rather see it the builds/prebuilds scoped as well. If I am working in PF4 land I shouldn't need to wait on PF3 to build, and vice versa. I think that is something that could be iterated on though.

[seanforyou23]

I'm learning some valuable history here :)

If I am working in PF4 land I shouldn't need to wait on PF3 to build, and vice versa.

Agree with this. Unfortunately I noticed that there were lots of 404's on font references if you just ran yarn && yarn bootstrap && yarn build then start up pf4, this is why I added the last step of yarn build:docs in the readme, because after that runs, the font references work just fine.

That leads me to believe there is still some kinks to work out regarding the separation of build/package scripts. Iterative approach sounds like a good path forward. I really want to consider and act on the other points raised here, but maybe for this issue it's enough to just recommend a functioning set of steps in the readme and we can optimize the build as part of another issue?

Just trying to get this to a stopping point before I go on vacation for a few days :)

[priley86]

I think another potential route to consider is adding another package script for lerna to target, ie “lerna run build:pf3” which could run those packages builds subscribing to that script. This is really a matter of preference I think. I attempted to logically split into prebuild/build/postbuild so that each package could subscribe to those steps and run them in parallel (a nice feature Lerna offers). I like to avoid daisy chaining where possible, but sometimes it’s necessary. Again, these are just important things to think through as we think about a modular package release process. The current build is modular but we could definitely add more targeted build scripts for scenarios like this.

For now, I’m 100% ok with simply updating the documentation like you’ve done though ;)

[seanforyou23]

We can always stand to improve the build - with the immediate issue resolved by tweaking the docs and there being plenty of other fun things to work on I'm inclined to close this and revisit if/when it becomes a problem again. The main thing was really to make sure following our first few steps didn't end in errors. Feel free to reopen if need be!