-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Asset hash caching #15054
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
Asset hash caching #15054
Conversation
* master: scalafmt remove last dep to irc remove deps to game & irc remove deps lobby->playban & bot->lobby remove pool dep to playban remove report dependency to playban remove round dep to history remove round dep to analyse remove clas dep to msg remove tutor dep to fishnet remove round dep to fishnet fix socket Env init order class UserLagCache(using Executor) class SocketRequester(using Executor) increase cache size fix relation types in app/ proper Relation type, move types around in core move AskPipeline and future extensions to scalalib
* master: (23 commits) remove lobby dep to game core.game.Blurs.nb & tweaks ignore study tests relying on game stubs #TODO fix me opaque type evalCache.Id rename _clock to computedClock remove importer module remove deps to game use normalized BinaryFen as evalCache id (lichess-org#15020) remove pool dep to game remove deps to game remove simul dep to game scalafmt Fix relation check on challenges Code golf, remove nested match Fix ignored test by adding emt for two moves Code golf Fix current clock calculation Add some emt parser tests Use emt when missing clock in PgnImport Use emt when missing clock in NewPgnImport ...
| moreCss = cssTag("analyse.practice"), | ||
| modules = List(analyseNvuiTag), | ||
| pageModule = PageModule( | ||
| "analysisBoard.study", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fun fact, back in the days I had to rename our analysis/analyse JS assets to analysisBoard, to prevent them from being filtered by ad-blockers.
I think it would be safer to stick to it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'll need to name the source file in ui/analyse analysisBoard.ts then due to the naming restrictions imposed by esbuild on content hashed assets. It's complicated why this is necessary but it is. That OK?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if we ran with analyse.23981abc.js to see if that problem still exists, and if we see evidence of ad-blockers misbehaving we'll just rename to analysisBoard.iwef9032.js or definitely-not-an-ad.ije2738s.js and deploy the changed assets?
The reason why I'm annoying about this is the main entry point of each module is named after the module folder since we can no longer remap output filenames for each entry point independently. And we'd need to violate that (in just this one case), or get used to paths like ui/analysisBoard/src/study/relay/relayTourView.ts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok let's try it as is
…into asset-hash-caching * 'asset-hash-caching' of https://github.com/schlawg/lila: remove craplet in package.json
* master: let kids read lichess blog posts - closes lichess-org#15092 fix perf stats - closes lichess-org#15096 leaner core.game Revert annotator test Make GameToRoot an object disable eslint for historic file remove unused game.UserInfo more core.game refactors move methods of out core.game to where they're used
|
So about the themeing aspect of this PR, I see ui/common/css/abstract/_theme.scss @mixin metal-bg {
@include to-bottom(hsl($site-hue, 7%, 22), hsl($site-hue, 5%, 19));
html.transp & {
@include to-bottom(hsla(0, 0, 0, 0.4), hsla(0, 0, 0, 0.5));
}
html.light & {
@include to-bottom(hsl(0, 0, 96) 0%, hsl(0, 0, 93));
}
}
@mixin metal {
@include to-bottom(hsl($site-hue, 7%, 22), hsl($site-hue, 5%, 19));
html.transp & {
@include to-bottom(hsla(0, 0, 0, 0.4), hsla(0, 0, 0, 0.5));
}
html.light & {
text-shadow: 0 1px 0 $c-font-shadow;
@include to-bottom(hsl(0, 0, 96) 0%, hsl(0, 0, 93));
}
}resulting in compiled CSS: .btn-rack__btn,.btn-rack form,#friend_box .friend_box_title,.button.button-metal,.button.button-empty:not(.disabled):hover,.button.button-empty.button-green:not(.disabled):hover,.button.button-empty.button-red:not(.disabled):hover {
background: linear-gradient(to bottom, hsl(37, 7%, 22%), hsl(37, 5%, 19%) 100%)
}
html.transp .btn-rack__btn,html.transp .btn-rack form,.btn-rack html.transp form,html.transp #friend_box .friend_box_title,#friend_box html.transp .friend_box_title,html.transp .button.button-metal,html.transp .button.button-empty:not(.disabled):hover {
background: linear-gradient(to bottom, hsla(0, 0%, 0%, 0.4), hsla(0, 0%, 0%, 0.5) 100%)
}
html.light .btn-rack__btn,html.light .btn-rack form,.btn-rack html.light form,html.light #friend_box .friend_box_title,#friend_box html.light .friend_box_title,html.light .button.button-metal,html.light .button.button-empty:not(.disabled):hover {
text-shadow: 0 1px 0 var(--c-font-shadow);
background: linear-gradient(to bottom, hsl(0, 0%, 96%) 0%, hsl(0, 0%, 93%) 100%)
}on most pages I count more than a hundred of unused |
|
Yes, there's a lot of unused selectors generated by those mixins/extends. We may be able to get rid of a lot of that, but I don't think it would buy us anything. Matching selectors is a tiny slice of the time spent rendering compared to interpreting the rules and doing layout. Make sure you compare render performance on the same host, I don't think you can look at lighthouse results from prod and compare them to lichess.dev. You'd need to checkout a commit prior to the theming on lichess.dev and run lighthouse there to get a good comparison of scores pre and post theme |
|
So it does mean that all themes will be loaded for everyone, and that adding a new theme adds weight and selectors to every page. That won't encourage me to add a plethora of new themes. |
|
Most new themes would just be different color definitions and make theme-all.css a bit bigger. If we needed different css, such as gradients, we'd need to make a separate rule in css. But that's optional. I think for 99% of any themes we would add, we can just let the default gradient happen or slap the light/transp classes on as well and use those. |
|
And yes, the idea is to keep all themes loaded for everyone. At least while the compressed size of all the css is comparable to what we had before (filtered by .ltr.dark.) |
|
Opera 55 is broken too. First version that works is Opera 72. |
|
Oh yeah, we can probably fix all of that. |
|
That oughtta do it |
| if (existing) { | ||
| if (i === ctrl.fork.selected()) { | ||
| existing.brush = brush; | ||
| existing.modifiers ??= {}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought typescript was rewriting these for us already, wasn't it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
esbuild doesn't run typescript. it just rips off the types, shits down their necks, and then calls exit(0)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
srs tho, that stuff may indeed get rewritten by esbuild. the problem was in javascipt that i was embedding directly in client manifest.xxxxxxxx.js and also in layout.scala
we could probably safely use it in typescript code. i don't really mind either way.
ui/build --update -cis necessary here. This PR also contains #14993ui/buildmakes two manifests, one for server, and one for client.ts)[dir]/[name].[hash].[ext]main.ts(entry point) sources have been renamed to<host-module>.tsto maintain traceability of output files under the pattern system but keep a flat asset folder.<host-module>.<filename>.tsui/pageletsrenamed toui/bitsaddress #14944