Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add an option to skip webgui builds #685

Open
wbqpk3 opened this issue Nov 17, 2023 · 3 comments
Open

Add an option to skip webgui builds #685

wbqpk3 opened this issue Nov 17, 2023 · 3 comments
Labels
Kind: Enhancement 🌟 Target: Developer environment Developer environment issues consist of CodeCompass or 3rd-party build tooling, configuration or CI. Target: WebGUI Issues related to the web frontend.

Comments

@wbqpk3
Copy link
Collaborator

wbqpk3 commented Nov 17, 2023

Whenever we execute make install it rebuilds the webgui despite no changes in webgui source code. We should add a CMake option to skip webgui builds for both webgui and webgui-new.

Relevant output of make install:

Initiating the Dojo Build System...
[...]
The Dojo Build System finished.
> [email protected] thrift-ts
> thrift-typescript --fallbackNamespace none

Generating TypeScript files from Thrift finished.
Building React App...

> [email protected] build
> next build && next export
@wbqpk3 wbqpk3 added Kind: Enhancement 🌟 Target: Developer environment Developer environment issues consist of CodeCompass or 3rd-party build tooling, configuration or CI. Target: WebGUI Issues related to the web frontend. labels Nov 17, 2023
@mcserep
Copy link
Collaborator

mcserep commented Nov 17, 2023

While installation should as fast as possible, we should not support easily broken installations, like not installing the webgui and leaving a previously built, possibly outdated version there.

Instead, an incremental installation of the webgui should be supported. This requires two steps:

  1. Only perform npm install, if the package-lock.json file changed. (For the old GUI, there is only a package.json file.) This is already done.
  2. Do not make a full rebuild of the web GUI, but an incremental build. NextJS supports ISR (Incremental Static Regeneration), though I am not sure how much work it would require to implement it. I would not waste time doing this with the old GUI, unless it is very easy to do.

Alternatively we could use the COMPONENT argument of the install() command in CMake to define multiple installation components. Then it would be possible to install the logger and the parser component and not install the webserver component, if it is not needed in a scenario.

@wbqpk3
Copy link
Collaborator Author

wbqpk3 commented Nov 20, 2023

Instead, an incremental installation of the webgui should be supported.

I agree, that would indeed be a better option.

Would that be possible to build the webgui during make and check if the webgui source code changed? In that case, make install would just simply copy the built files.

@mcserep
Copy link
Collaborator

mcserep commented Nov 29, 2023

The build workflow of Node is not highly compatible with CMake, be it is nice that we have a unified build system for the complete project.

While we can build the frontend during the build stage of CMake, this will raise another performance issue during development. If someone is working on both the GUI and backend Thrift service, he/she would need to rebuild the frontend each time a make command is issued. This is vain, as during development, usually not a statically built version of the frontend is tested, but the development server of NodeJS is utilized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Kind: Enhancement 🌟 Target: Developer environment Developer environment issues consist of CodeCompass or 3rd-party build tooling, configuration or CI. Target: WebGUI Issues related to the web frontend.
Projects
None yet
Development

No branches or pull requests

2 participants