Merge behave-flow as a package @behave-graph/react-flow

[bhouston]

@beeglebug

This does not yet build.

[bhouston]

@oveddan @beeglebug I do not know how to configure preconstruct to compile the TSX files in the @behave-graph/react-flow package. So it is erroring out there and I am at least temporarily stuck.

[oveddan]

You need to have babel use the @babrel/preset-react plugin. If you want it to only be used in that package, you would need to have a babel config in that package folder.

See the .babelrc file in my branch:
https://github.com/oveddan/behave-graph/tree/behave-flow/packages/flow

Youre going to run into other issues with the css files, which I discussed in the other PR; not sure the best solution for that one.

[oveddan]

you don't need to import react if you have the right typescript settings.

You can add to the tsonfig (in compilerOptions):
"jsx": "react-jsx" and you get the newer JSX Transform which also has performance enhancements.

So I would recommend adding a tsconfig here that extends the root one ut adds that setting, and removing the import react everywhere.

Example:

{
  "extends": "../tsconfig.json",
  "compilerOptions": {
    "jsx": "react-jsx"
  },
  "include": ["./src"]
}

[beeglebug]

fwiw i'm still not convinced the package should be called @behave-graph/react-flow to me that heavily implies its a fork of react-flow

[beeglebug]

i've just checked out this branch to try and help get it working, but i cant for the life if me figure out how to get anything building, can we update the readme with some basic commands for people who are new to monorepos?

for example, every file in the packages/react-flow directory is complaining about no dependencies, because it has no node_modules folder, am i supposed to manually go into there and npm install, or should an install from the top level do that for me?

how do i get the react-flow package to know about the existence of the core package? some sort of symlinks? do i need to set those up myself?

Edit: Ignore all that, i forgot i was running an older version of npm, swapped to latest and the symlinks worked as expected 🤦

[beeglebug]

i've got almost everything working now, it all builds, and i can serve the graph-editor with npm run graph-editor

but now it dies because of css, and it looks like preconstruct just flat out doesnt support css (see preconstruct/preconstruct#268)

not sure how we move past this, i've tried css modules instead of tailwind, but the same problem happens

the only way i can think of for now is to manually create a css file and use global class names in the components, but that feels so janky

[beeglebug]

i've just noticed it also fails with a ReferenceError: React is not defined error when trying to load the vite server, i've added the vite react plugin which is supposed to handle the jsx transform, but it didnt work, and i've run out of time for this morning so will have to come back to it

[bhouston]

@beeglebug is there a different build system we could use rather than preconstruct? I guess we could go back to the more manual tsc+rollup system you had earlier?

[beeglebug]

it looks like everyone uses lerna https://npmtrends.com/@preconstruct/cli-vs-lerna

[oveddan]

Edit: Ignore all that, i forgot i was running an older version of npm, swapped to latest and the symlinks worked as expected 🤦

With yarn this wouldn't be an issue (maybe we should consider using that as the main build tool).

The issue isn't with preconstruct - preconstruct just calls babel to build the js and symlinks it with minimal configuration; nowhere in babel's documentation do they mention building css - because it's not really meant for that and you shouldn't really be bundling css with your js files (I haven't seen many packages that do that).

Both Reactflow, tailwind, and rainbowkit - method of css integration is by asking you to import the css. They all use a separate postcss from the babel build step to build the css, but nowhere in their framework javascript code do they import the css files.

Same thing can be done in this package.

See my PR where postcss is used to build the css and copy it to the dist folder.

[oveddan]

The one thing rollup would be able to do would be call postcss as a separate step from the babel build. You could do things this way but then would want to add the step to use lerna to symlink - and then symlinking across different folders in packages becomes tricky. preconstruct makes that all easy with minimal config

[beeglebug]

With yarn this wouldn't be an issue (maybe we should consider using that as the main build tool).

It wasnt really a yarn/npm thing, its a "me using an ancient version of npm for work" thing, we'd have had the same with yarn if i used one from before it had workspace support.

you shouldn't really be bundling css with your js files

Nobody has suggested doing that, we want to use css modules, but generate a separate css build file and tell users to include it (exactly as other packages do)

[beeglebug]

if the build system had support for css, then it could be done automatically in one step, we could use css modules in react and get all the benefits that come with that, instead of going old school and having two build steps

[oveddan]

if the build system had support for css, then it could be done automatically in one step, we could use css modules in react and get all the benefits that come with that, instead of going old school and having two build steps

yeah that could work...just requires setting up rollup. I've also seen this being used:
https://vanilla-extract.style/documentation/getting-started/

[beeglebug]

Doesn't preconstruct use rollup under the hood? It's a shame it's not fully configurable, there's multiple issues like the one I linked where they basically wash their hands of CSS.

…

On Thu, 24 Nov 2022, 15:01 Dan Oved, ***@***.***> wrote: if the build system had support for css, then it could be done automatically in one step, we could use css modules in react and get all the benefits that come with that, instead of going old school and having two build steps yeah that could work...just requires setting up rollup. I've also seen this being used: https://vanilla-extract.style/documentation/getting-started/ — Reply to this email directly, view it on GitHub <#166 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAHJAMDTFXTKF6RMXW5PMYTWJ57K7ANCNFSM6AAAAAASJIU73I> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[oveddan]

I remembered using leva before, which is a react based gui generator (also from the react three fiber folks) - and dug into their code. They use preconstruct, and for css use stitches, a nice css-in-js solution.

You can see their setup here

[oveddan]

Doesn't preconstruct use rollup under the hood? It's a shame it's not fully configurable, there's multiple issues like the one I linked where they basically wash their hands of CSS.

yes they do use rollup and it would be nice if you could configure it

[oveddan]

Here's a list of css-in-js frameworks, implementing one of them would probably solve this problem

[beeglebug]

I think at this stage I'd be more comfortable with a plain CSS solution rather than adopting a new CSS in JS framework. Really the amount of styles in this package is small, so it's a shame this is such a sticking point

…

On Thu, 24 Nov 2022, 15:18 Dan Oved, ***@***.***> wrote: Doesn't preconstruct use rollup under the hood? It's a shame it's not fully configurable, there's multiple issues like the one I linked where they basically wash their hands of CSS. yes they do use rollup and it would be nice if you could configure it — Reply to this email directly, view it on GitHub <#166 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAHJAMBAT2SVVXUC2YQVRQDWJ6BMLANCNFSM6AAAAAASJIU73I> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[oveddan]

The fact that there is a minimal amount of css should make it easy to just declare it a different way, and a lot less work and code change than having to configure rollup for each package, and then postcss for this one. I'd be glad to work on this.

[beeglebug]

Ideally the compiled CSS for this package would contain the base react-flow stylesheet so we don't need to tell users to add two important theirs and ours. Any idea how we would go about that with a CSS in JS solution?

…

On Thu, 24 Nov 2022, 15:28 Dan Oved, ***@***.***> wrote: The fact that there is a minimal amount of css should make it easy to just declare it a different way, and a lot less work and code change than having to configure rollup for each package, and then postcss for this one. — Reply to this email directly, view it on GitHub <#166 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAHJAMHB2CHIVKZE2KSYR5TWJ6CQHANCNFSM6AAAAAASJIU73I> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[oveddan]

Ideally the compiled CSS for this package would contain the base react-flow stylesheet so we don't need to tell users to add two important theirs and ours. Any idea how we would go about that with a CSS in JS solution?

Yeah agreed - it would be nice if they can just import one stylesheet instead of having to import two. Will look into it.

[oveddan]

Having it as one css bundle could be done by declaring an index.css file and importing the reactflow styles and building it all with postcss.

reactflow uses a separate build step with postcss from their js build with rollup so that could be an option here.

[oveddan]

shouldn't the graph-editor example have a package.json?

[bhouston]

I made a few more changes and I have it loaded up in the graph-editor project with a package.json. Obviously missing CSS.

Bad things:

1. I added a "import React from "react" to Modal.tsx. It isn't used in the file itself but it appears that it is needed for site to load. I am unclear what is going on.
2. CSS is obviously not loading.

I am definitely not an expert at any of this build stuff. help would be appreciated. I guess I can merge this and we can look to fix it going forward.

[bhouston]

Thank you @oveddan and @beeglebug for your help on this PR. I know it was a challenge.