DEV: load styleguide assets only when needed (PR #10918)

GitHub

@davidtaylorhq

I have two questions on this:

  • Would this be impacted by AnonymousCache, I don’t see how it could, but I don’t know all the details about AnonymousCache
  • Shouldn’t we do this for the admin bundle too ?

Would this be impacted by AnonymousCache, I don’t see how it could, but I don’t know all the details about AnonymousCache

The anonymous cache is keyed based on the path, so this should be fine :+1:

Shouldn’t we do this for the admin bundle too ?

We only load the admin bundle for staff:

https://github.com/discourse/discourse/blob/d77e31b7e946bcc64f097c81b7eab110187102d6/app/views/layouts/application.html.erb#L35-L38

BTW, I think this change will break navigating to /styleguide within ember. A full page reload will be required to access /styleguide. That could be worked around by adding styleguide to the SERVER_SIDE_ONLY list in discourse/lib/url.

BTW, I think this change will break navigating to /styleguide within ember. A full page reload will be required to access /styleguide. That could be worked around by adding styleguide to the SERVER_SIDE_ONLY list in discourse/lib/url. (might need to add a plugin API there…)

Yes that was my question about admin. FWIW I don’t think we have a link to ember, so you only reach it by going to /styleguide, so I think it’s fine.

But I would want to do the same for admin because even if you don’t go to admin pages we always load it for staff, if there was a clean and easy way to do it, that would be a small win.

    request.fullpath&.start_with?('/styleguide')

“SNOT”: safe navigation operator, try (it) :stuck_out_tongue_closed_eyes:

Should be double & actually, because request can be nil

If the request can be nil, then request.fullpath might fail, so should be request&.fullpath&.start_with...

FWIW I don’t think we have a link to styleguide, so you only reach it by going to /styleguide, so I think it’s fine.

If I link to /styleguide in a post, it will also break. That could be quite confusing?

FWIW I don’t think we have a link to styleguide, so you only reach it by going to /styleguide, so I think it’s fine.

If I link to /styleguide in a post, it will also break. That could be quite confusing?

yes sure there are plenty of ways to make it fail, and oe could also argue it should have a link in hamburger menu.

Will work in a PR for SERVER_SIDE_ONLY support in API

One thing I just thought… how is the plugin going to call the JS api… because its JS assets will not be loaded :exploding_head:

Maybe we should just add /styleguide to discourse/lib/url.js in core. This is a core plugin, so it’s probably ok.

One thing I just thought… how is the plugin going to call the JS api… because its JS assets will not be loaded :exploding_head:

Maybe we should just add /styleguide to discourse/lib/url.js in core. This is a core plugin, so it’s probably ok.

ahah yes I experienced this while trying to create a link in hamburger menu to test my other PR