[mod] theme/simple: refactor codebase #5073
Conversation
b31b56c to
98d5c0c
Compare
|
FYI: last force push was just a rebase on current master / except #5073 (comment) nothing has been modified by mine .. so far :-) The second force push was needed to fix an issue in the here modified script, issue was reported by shfmt from the rebase ..diff |
98d5c0c to
e09f356
Compare
|
The remote for this PR was the branch in #4988. Actually, all commits except the last one are duplicated. |
Yes, I now .. but I thought I can review this Draft (to not double the efforts for testing) .. so I did both PRs in one review ;-) |
e09f356 to
e0a4ee9
Compare
In the past we have bundled the JS files to save additional request/response cycles for the JS to load .. which is also the reason why we have constructs like theme_icons. But I also think that the number of endpoints cannot be compared to the number of icons. With icons, the saving of additional request/response cycles for small files is worthwhile. But with the (much smaller number of) endpoints, the coding becomes cleaner. Long story told short / I agree with your approach 👍 |
| "license": "AGPL-3.0", | ||
| "type": "module", | ||
| "scripts": { | ||
| "build": "npm run build:icons && npm run build:vite", |
There was a problem hiding this comment.
with my comment in mind: isn't it better we lint before build?
We could implement a lint here ... or in the lib_sxng_vite.sh .. or in the lib_sxng_thems.sh .. whatever you think is the best place.
Since only a few developers have worked on the client so far, we are free to design the developer workflows as we see fit and can make changes without looking back.
We should design them in the same way as is customary in JS/TS development.
The shell scripts (and the Makefile) are just wrappers that should provide simplified access to the developer workflows without creating additional complications.
@inetol what would you suggest to implement the developer workflows?
(For example, only performing the build after all tests have been completed.)
There was a problem hiding this comment.
We could do this under themes.all and check the lint there before building. I'm not too convinced on mixing different actions (like themes.simple having the linting process), as lint is not always a hard requirement to successfully build.
themes and vite wrappers are redundant and confusing rn, but are not the scope of this PR.
ef34245 to
4492753
Compare
|
FYI: last force push was a rebase on branch master / no code changes from my side .. |
| const isScrolling = scrollTop >= 100; | ||
| resultsElement.classList.toggle("scrolling", isScrolling); |
There was a problem hiding this comment.
Not related to this PR (same issue in current master branch): When infinite_scroll: is active, the back to top button is never shown / especially in infinitely long result lists such a button would be convenient.
update: back to top button is shown in the desktop view but not on smaller devices / below (max-width:79.75em).
Lay the foundation for loading scripts granularly depending on the endpoint it's on. Remove vendor specific prefixes as there are now managed by browserslist and LightningCSS. Enabled quite a few rules in Biome that don't come in recommended to better catch issues and improve consistency. Related: - searxng#5073 (comment) - searxng#5073 (comment)
a8eccf6 to
6e36a50
Compare
return42
left a comment
There was a problem hiding this comment.
I pushed 3 commits with some nits on top and I modified the commit to have all modification on files in folder searx/static in a final [build] /static commit.
There is an issue in the CI
I guess its not related to this PR .. but I will have a look to see whats wrong with this unit test .. could take a while, I can't reproduce the issue on my desktop!?!
This issue was fixed in #4949, idk why appeared again |
Python's |
|
If it's an upstream issue mark every related test as soft fail and we'll revisit this later |
OK, changes look good to me. Feel free to merge this when ready |
TypeScript is a superset of JavaScript, converting the entire theme to TypeScript allows us to receive much more feedback on possible issues made in package updates or our own typos, furthermore, it allows to transpile properly to lower specs. This PR couldn't be done in smaller commits, a lot of work needed to make everything *work properly*: - A browser baseline has been set that requires minimum **Chromium 93, Firefox 92 and Safari 15** (proper visuals/operation on older browser versions is not guaranteed) - LightningCSS now handles minification and prefix creation for CSS. - All hardcoded polyfills and support for previous browser baseline versions have been removed. - Convert codebase to TypeScript. - Convert IIFE to ESM, handling globals with IIFE is cumbersome, ESM is the standard for virtually any use of JS nowadays. - Vite now builds the theme without the need for `vite-plugin-static-copy`. - `searxng.ready` now accepts an array of conditions for the callback to be executed. - Replace `leaflet` with `ol` as there were some issues with proper Vite bundling. - Merged `head` with `main` script, as head was too small now. - Add `assertElement` to properly check the existence of critical DOM elements. - `searxng.on` renamed to `searxng.listen` with some handling improvements.
Set minor versioning for most of the packages that iterate fast or we know won't cause problems, and fixed versioning for the rest. Packages going into bundles should be placed in "dependencies". The inspection of prod bundles is necessary. Although it does not make a lot of sense right now, it will be useful in later PR and will give us a reference to start with.
From `mod-simple-strict`
Lay the foundation for loading scripts granularly depending on the endpoint it's on. Remove vendor specific prefixes as there are now managed by browserslist and LightningCSS. Enabled quite a few rules in Biome that don't come in recommended to better catch issues and improve consistency. Related: - searxng#5073 (comment) - searxng#5073 (comment)
SPDX short-form identifiers to communicate license information in a simple, efficient, portable and machine-readable manner [1] [1] https://spdx.dev/learn/handling-license-info/
73557a7 to
f822aa3
Compare
Lay the foundation for loading scripts granularly depending on the endpoint it's on. Remove vendor specific prefixes as there are now managed by browserslist and LightningCSS. Enabled quite a few rules in Biome that don't come in recommended to better catch issues and improve consistency. Related: - #5073 (comment) - #5073 (comment)
Done :-) .. Thank you very much (also for your patience with me) .. that was really great work 🚀 |
Since searxng/searxng#5073 we add a script directly to the [`base.html`](https://github.com/searxng/searxng/blob/master/searx/templates/simple/base.html), we need `'unsafe-inline'`.
| <script> | ||
| // update the css | ||
| document.documentElement.classList.remove('no-js'); | ||
| document.documentElement.classList.add('js'); | ||
| </script> |
There was a problem hiding this comment.
The inline script needs to be removed from the HTML, see
To avoid an `unsafe-inline` in the CSP header, the JS code must be moved to the client side [1]. The `<script>` tag at the end of the HTML originates from the old implementation of the JS client. Since PR-5073 [2] was merged, the `type` is now `module`, and the tag must be moved to the beginning of the HTML. > We need to inline this "JS is enabled?" thing to prevent layout shifts and > temporary "no JS enabled" visuals as ESM scripts loads and evals everything > deferred from initial DOM render [3] That's true in theory, but in practice, this effect is unnoticeable because it's masked by another effect (which we can't avoid): If we load the page with a severely throttled connection, the HTML (result list) takes a long time to load. Then the CSS is loaded, which also takes longer. Until the CSS has loaded, there's no layout. A layout shift is therefore largely determined by the loading of the HTML and CSS itself. The running times of the ESM script can be neglected compared to the loading times of HTML & CSS. [1] searxng/searxng-docker#424 (comment) [2] searxng#5073 [3] searxng/searxng-docker#424 (comment)
To avoid an `unsafe-inline` in the CSP header, the JS code must be moved to the client side [1]. The `<script>` tag at the end of the HTML originates from the old implementation of the JS client. Since PR-5073 [2] was merged, the `type` is now `module`, and the tag must be moved to the beginning of the HTML. > We need to inline this "JS is enabled?" thing to prevent layout shifts and > temporary "no JS enabled" visuals as ESM scripts loads and evals everything > deferred from initial DOM render [3] That's true in theory, but in practice, this effect is unnoticeable because it's masked by another effect (which we can't avoid): If we load the page with a severely throttled connection, the HTML (result list) takes a long time to load. Then the CSS is loaded, which also takes longer. Until the CSS has loaded, there's no layout. A layout shift is therefore largely determined by the loading of the HTML and CSS itself. The running times of the ESM script can be neglected compared to the loading times of HTML & CSS. [1] searxng/searxng-docker#424 (comment) [2] searxng#5073 [3] searxng/searxng-docker#424 (comment)
To avoid an `unsafe-inline` in the CSP header, the JS code must be moved to the client side [1]. The `<script>` tag at the end of the HTML originates from the old implementation of the JS client. Since PR-5073 [2] was merged, the `type` is now `module`, and the tag must be moved to the beginning of the HTML. > We need to inline this "JS is enabled?" thing to prevent layout shifts and > temporary "no JS enabled" visuals as ESM scripts loads and evals everything > deferred from initial DOM render [3] That's true in theory, but in practice, this effect is unnoticeable because it's masked by another effect (which we can't avoid): If we load the page with a severely throttled connection, the HTML (result list) takes a long time to load. Then the CSS is loaded, which also takes longer. Until the CSS has loaded, there's no layout. A layout shift is therefore largely determined by the loading of the HTML and CSS itself. The running times of the ESM script can be neglected compared to the loading times of HTML & CSS. [1] searxng/searxng-docker#424 (comment) [2] #5073 [3] searxng/searxng-docker#424 (comment)
This is one of various PR to refactor the simple theme internally.
TypeScript is a superset of JavaScript, converting the entire theme to TypeScript allows us to receive much more feedback on possible issues made in package updates or our own typos, furthermore, it allows to transpile properly to lower specs. This PR couldn't be done in smaller commits, a lot of work needed to make everything work properly:
From #4988:
vite-plugin-static-copy.searxng.readynow accepts an array of conditions for the callback to be executed.leafletwitholas there were some issues with proper Vite bundling.headwithmainscript, as head was too small now.assertElementto properly check the existence of critical DOM elements.searxng.onrenamed tosearxng.listenwith some handling improvements.From #5073:
Visual from baseline browsers:
Chromium 93
Search:
Images: (2nd review; image fallback was not working properly)
Maps:
Preferences:
Firefox 92
Search:
Images:
Maps:
Preferences:
Safari 15
Search:
Images:
Maps:
Preferences:
