Skip to content

Conversation

@schlawg
Copy link
Contributor

@schlawg schlawg commented Apr 7, 2024

ui/build --update -c is necessary here. This PR also contains #14993

  • let filenames do cache busting
  • lila may be restarted without invalidating client assets
  • hard reload is no longer needed. mobile is easier to test
  • ui/build makes two manifests, one for server, and one for client
  • manifests map asset keys to content hashes to build asset filenames
  • asset keys are the name of the asset prior to the hash component - the complete basename of the original typescript entry point (excluding .ts)
  • server preloads script dependencies
  • code splitting now mandatory, -s option gone from ui/build
  • esbuild hashing requires output filenames follow a static pattern [dir]/[name].[hash].[ext]
  • using [dir] to distinguish things would recreate the source folder hierarchy in our assets folder, we don't want this.
  • therefore, a lot of main.ts (entry point) sources have been renamed to <host-module>.ts to maintain traceability of output files under the pattern system but keep a flat asset folder.
  • for modules with multiple entry points - the input sources are renamed as <host-module>.<filename>.ts
  • ui/pagelets renamed to ui/bits
  • css is hashed independently by ui/build with node's sha-1 implementation, which is plenty fast

address #14944

schlawg and others added 26 commits March 29, 2024 20:12
* 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
@schlawg schlawg requested a review from niklasf April 8, 2024 00:41
@schlawg schlawg requested review from fitztrev and ornicar April 8, 2024 00:47
schlawg and others added 2 commits April 14, 2024 09:58
* 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",
Copy link
Collaborator

@ornicar ornicar Apr 15, 2024

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.

Copy link
Contributor Author

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?

Copy link
Contributor Author

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

Copy link
Collaborator

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

ornicar and others added 12 commits April 15, 2024 16:49
…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
@ornicar
Copy link
Collaborator

ornicar commented Apr 17, 2024

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 html.light or html.transp selectors. Does that mean we're adding even more for each theme that we would add?

@schlawg
Copy link
Contributor Author

schlawg commented Apr 17, 2024

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

@ornicar
Copy link
Collaborator

ornicar commented Apr 17, 2024

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.

@schlawg
Copy link
Contributor Author

schlawg commented Apr 17, 2024

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.

@schlawg
Copy link
Contributor Author

schlawg commented Apr 17, 2024

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.)

@ornicar
Copy link
Collaborator

ornicar commented Apr 17, 2024

Not about themes:
our readme currently says that Safari 11.1 is supported. And indeed lichess.org works on Safari 11.1, I just checked on browserstach.
But lichess.dev doesn't. At all.
257x2864_2024-04-17
Safari 12.1 doesn't work either. And neither does 13.1.
The first version that works is 14.1. That's a big jump. We'd be looking at thousands of players thrown out.
Is there a way to avoid that?

@ornicar
Copy link
Collaborator

ornicar commented Apr 17, 2024

Opera 55 is broken too. First version that works is Opera 72.
Also Firefox 67 -> 79.
Edge 91 still works.

@schlawg
Copy link
Contributor Author

schlawg commented Apr 17, 2024

Oh yeah, we can probably fix all of that.

@schlawg
Copy link
Contributor Author

schlawg commented Apr 17, 2024

That oughtta do it

if (existing) {
if (i === ctrl.fork.selected()) {
existing.brush = brush;
existing.modifiers ??= {};
Copy link
Collaborator

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?

Copy link
Contributor Author

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)

Copy link
Contributor Author

@schlawg schlawg Apr 17, 2024

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.

@ornicar ornicar merged commit fc7f889 into lichess-org:master Apr 17, 2024
@schlawg schlawg deleted the asset-hash-caching branch April 17, 2024 16:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants