Refactor frontend

[hannaseithe]

Short description

This refactor provides a pattern for further frontend refactoring. Ts files are divided into template files and feature modules. Furthermore we now automatically generate a documentation for the frontend through integration of typedoc with our sphinx documentation for the backend files.

The goal of this refactor (and the subsequent refactoring of all feature modules) is/will be:
1.) To make it clear which ts file are used in which template (and vice versa)
2.) To reduce the amount of selector methods we crawl the DOM for
3.) Scope feature modules to a specific root element, to reduce the chance of accidentally attaching an eventListener to the wrong element
4.) To scope template modules to one specific template only

Proposed changes (main ones)

• a node script generateRegistry.ts (as npm script: generate:registry) that generates a file registry.ts which contains a record of registered feature modules and make it part of the build process
• a bootstrapModules function in index.ts that dynamically imports and initializes feature modules that are need for the current page
• search-query.ts refactored as a feautre module and data-js-search-query attribute added on root element in template
• all TS files that are only used in one template moved to the /template folder and set as entry-points in webpack.config.json and then added the respective bundles in the templates
• delete all imports from index.ts that are template modules or feature modules
• create a npm script docs:frontend that uses typedoc to generate *.md documentation files for the ts files and then use the myst-parser plugin to generate *.rst files with sphinx as part of our documentation

Side effects

• if a template ts file is actually also used in another template (than the one I identified), it will be missed there. I have tried to be very thorough in my search, but I still might have missed one or two
• by using more entry points than before (for the template modules), the frontend will have to do one additional fetch for one more bundle

Faithfulness to issue description and design

There are no intended deviations from the issue and design.

How to test

• basically the whole frontend with its functionalities needs to be tested for all template modules and the one feature module. Those are the files inside:
  • integreat_cms/static/src/js/template/
  • integreat_cms/static/src/js/feature/
• make sure that the correct bundles are included for the templates affected and that the content of the bundles is also correct
• see that the registry.ts Generation works if you register a new feature module (with npm run build)
  • make sure the validation works during generation by adding a "wrong" feature module (with wrong pattern)
• make sure the bootstrapModules function works during runtime
  • check that it catches misspelled data-js- attributes
  • add a wrong registry.ts entry and make sure the errors are logged
• generate the documentation with tools/make_docs.sh and see that it works

Next steps for Refactor

• All ts files inside the js folder that work based on eventListeners should be turned into feature modules (that excludes exported utility functions and the files that create preact components). This includes the following:
  • refactor and add the following lines (analogue to search-query.ts) to the code:
    - export const moduleName = <name-of-module>;
    - const init = (root:HTMLElement) => { //register all eventListeners on or inside the root Element }
    - export default init;
  • move the file into the folder /features
  • attach the attribute data-js-<moduleName> to the root element of the feature for wherever it is needed in the templates
  • run tools/run.sh at least once to generate the new registry.ts file
• use htmx wherever we fetch from the backend
• use webpack-bundle-analyzer to consider if we could get rid of big external libraries that are not essential for us or at least only partially bundle them, to reduce bundle size
• create automatic frontend tests

Resolved issues

Relates to: #2684

---

Pull Request Review Guidelines

[JoeyStk]

Thank you very much for this PR! I think it's a great step forward for us in helping us keeping our codebase clean 🚀 I think you did a wonderful job 💛

This isn't a full review yet, but I already wanted to give you some preliminary feedback :). After running the server, I get the following error in the web console.

Details

Uncaught (in promise) DOMException: Document.querySelectorAll: '[data-js-*]' is not a valid selector
_callee2 index.ts:103
i main.51245facc06e27bf6459.js:557
o main.51245facc06e27bf6459.js:558
__awaiter main.51245facc06e27bf6459.js:587
__awaiter main.51245facc06e27bf6459.js:569
bootstrapModules index.ts:100
ts index.ts:163
ts index.ts:159
Webpack 5
main.51245facc06e27bf6459.js:687
__awaiter main.51245facc06e27bf6459.js:569
bootstrapModules index.ts:100
ts index.ts:163
(Async: EventListener.handleEvent)
ts index.ts:159
Webpack 5

[hannaseithe]

This isn't a full review yet, but I already wanted to give you some preliminary feedback :). After running the server, I get the following error in the web console.

Thank you for catching this. I added this at the end and did not properly check. Unfortunately there are no wildcards for attribute selection the way I imagined it ([data-js-*]) - see also here: https://stackoverflow.com/questions/21222375/css-selector-for-attribute-names-based-on-a-wildcard, which means I will not be implementing this specific runtime check (where I wanted to check if a data-js-* was added that does not exist in the registry), as the only other option (iterating over all nodes) is not efficient imo.

[JoeyStk]

Still not a full review, but here is one thought of mine :)

[JoeyStk]

Suggested change

	The generation process is divided into three stages:
	The generation process is divided into four stages:

I think this should be four, right? At least we list four steps below :)

[JoeyStk]

Additionally I think it might help to rebase soon, but it's probably not going to be a nice experience 😬

[jonbulz]

My two cents:
In general, I like the idea of having a registry.ts as source of truth. But I'd strongly suggest that we decide on one of two possible paths:

• 1. We auto-generate the registry, but don't commit the file
• 2. We commit the file, but in a human-readable form. If the file is still generated by some tool or automation, we need to make sure that the generated file is identical across different environments and operating systems. Alternatively, we could edit the registry.ts by hand if a new module is added (which shouldn't happen all that often)

I personally lean towards option 2 and "edit-by-hand", unless there is a very easy way to generate the file. If we want to ensure that the registry does not contain some typo or other mishap, we can add a pre-commit hook and/or a CI job that checks that.

In general, I think you did a great job in making the frontend more structured and maintainable! Thank you very much! :)

I still didn't get around to investigate the way you implemented what you call "template modules", but the "feature modules" are a good and useful abstraction IMO

[hannaseithe]

I personally lean towards option 2 and "edit-by-hand", unless there is a very easy way to generate the file. If we want to ensure that the registry does not contain some typo or other mishap, we can add a pre-commit hook and/or a CI job that checks that.

Yes, I also tend towards getting rid of the automatic generation but making it human readable, especially as the validation logic I used in the node script for the automatic generation could be re-used in the pre-commit and/or CircleCI job.

The registry form I envision:

export const registry: Record<string, string> = {
  "search-query": "./js/feature/search-query",
  }

[hannaseithe]

Additionally I think it might help to rebase soon, but it's probably not going to be a nice experience 😬

Thank you very much for that tip :) I will keep doing regular rebases until merged