Glimpse v2 - Bundle registered client scripts - WIP

[MisterJames]

The idea here is to wrap up like-priority extension scripts into single files.

Based on Glimpse.Core.Extensibility.ScriptOrder enum, build a bundle for each section of the document. Rather than rendering out a script tag for each script, a single, bundled file would be injected in their place.

A possible further optimization would be to detect for a count of one and write out the raw script if there only exists one to save on serialization costs.

Open to conversation, suggestions, pointers, gotchas or insights anyone has to offer.

[avanderhoorn]

Great idea. Lets make it happen. Just let us know if/when you have any questions. Also once you have a design in mind, let us know and we can help provide feedback. But I'm guessing that its all going to be pretty straight forward.

[MisterJames]

PoC is working, injecting an eensy bit of JS for each client script resource request now. Starting on returning requested client script for-realz, yo.

[avanderhoorn]

Looking great! Keep up the work.

In terms of the work, I think we might run into a small issue that we should work through. Specifically the current output looks like the following:

<script type='text/javascript' src='/glimpse.axd?n=glimpse_clientscripts&order=ClientInterfaceScript&glimpse_request_id=d1f4bdcf-3d29-450f-9723-5605f2d444e8'></script>
<script type='text/javascript' src='/glimpse.axd?n=glimpse_clientscripts&order=RequestMetadataScript&glimpse_request_id=d1f4bdcf-3d29-450f-9723-5605f2d444e8'></script>
<script type='text/javascript' src='/glimpse.axd?n=glimpse_clientscripts&order=RequestDataScript&glimpse_request_id=d1f4bdcf-3d29-450f-9723-5605f2d444e8'></script>

Putting aside the fact that the hash is missing and that later one we should use URI templeates, because the glimpse_request_id is being included on every request, we aren't able to cache these as the requestId changes on every request. The only script that really needs the glimpse_request_id is RequestDataScript.

So this leads me to wonder if we should have specific logic for the RequestDataScript or create an interface called something like IExplicitClientScript, which, when the GlimpseScriptTagsGenerator hits an IClientScript that implements this, it calls IExplicitClientScript.GetUri(...) on that script and renders out a specific script tag for that script and excludes it from the merge.

Alternatively we could have a mechanism which targets at ScriptOrder level and allows you to take control of the rendering of a given ScriptOrder value - in this case RequestDataScript would have a custom rendering of its tag but the others would use the default glimpse_clientscripts based strategy.

The different between these approaches is one is targeted at a script level and the other is targeted at the order level. I personally like the second option better but it gets slightly complicated if you have multiple scripts registered to RequestDataScript.

@nikmd23 do you have any thoughts.

[nikmd23]

So this leads me to wonder if we should have specific logic for the RequestDataScript or create an interface called something like IExplicitClientScript

Specific logic is what recommend. ClientScript implementations should not have any more say than they currently do about their URI, since that is really up to the ResourceEndpointConfiguration at the end of the day anyways. (Which might not choose to use UriTemplate or QueryString Parameters)

[MisterJames]

Okay, so if we're going to go the specific handling route, and at the risk of underengineering this, couldn't I just switch on the order enum when generating the URI?

foreach (var scriptGroup in scripts)
{
    switch (scriptGroup.Key)
    {
        case ScriptOrder.RequestDataScript:
            // do specific case
            break;
        default:
            // all other cases
    }
}

Again, still getting familiar with the code, so if I'm overlooking something please feel free to correct. This wouldn't handle the case of wanting a script excluded from the bundle (is that needed at this point?) but it at least lets us handle the case where a specific URI is needed for any given (internal) script.

I had also asked about the possibility of creating a second enum, one for the script order enums that shouldn't be added to by extension authors (the enum is mixed internal/external right now). Though I would implement it on a separate PR, it would mean that we'd need to back off the order-by-enum approach and implement specific logic for each collection in the the order enums anyway.

@avanderhoorn, I apparently didn't write down in my notes where the framework provides the mechanism for generating a hash. Could you point me in the right direction?

[MisterJames]

Comment right beside comment and close is a bad idea 😒

[avanderhoorn]

Ya what you have with the switch statement looks great and will work well for now. We should also probably add ClientInterfaceScript and RequestMetadataScript as well. This means people can't muck around with it.

Also I agree that we don't need to worry about the excluding scripts. For now lets keep the one enum and we can see about splitting up post getting this going a bit more. I've got some ideas how we can do that but it won't effect the core work we are doing.

[nikmd23]

Comment right beside comment and close is a bad idea 😒

No 💩 right!? I've done that 💯 times before.

I agree with @avanderhoorn - the implementation you have is perfectly fine.

[MisterJames]

Okay...so the difference here is that at this point, I'm not in the same kind of "know" that a FileResource is. Here's a "for instance".

Normally, it looks like the glimpse.js file would be setup as a ClientResource (of :FileResource) with the internal name "glimpse_client" and able to get at the file because of properties set in the constructor. It then reads the stream, etc., and pushes out a new FileResourceResult wrapped with a CacheControlDecorator. Sweet beans.

I know that I can find the resource based on the name, like so:

var resource = resources.FirstOrDefault(r => r.Name.Equals(path, StringComparison.InvariantCultureIgnoreCase));

Now, if I wanted to go this way to get at the file deets:

 var clientResource = resource as ClientResource;
 if (clientResource != null)
 {
     var info = clientResource.GetEmbeddedResourceInfo(context);
 }

...then I'm still stuck, because that won't actually work (GetEmbeddedResourceInfo is not visible due to access level here). I do get clientResource as a ClientResource, but can't get at the pretty bits.

I'm supposed to be returning an IResourceResult here, so can I just do one of these:

  var clientResource = resource as ClientResource;
  if (clientResource != null)
  {
      return clientResource.Execute(context);
  }

Anyhoo...plugged away a bit for tonight, hoping you gents (@nikmd23 and @avanderhoorn) may have some thoughts.

[MisterJames]

Unicorns and rainbows. You can just execute the resource.

We have liftoff. UI is loading...no data yet, but my slow, slow mind is catching on...

[MisterJames]

I have hard-coded a couple of things in the script URIs at this point (I think callbacks should be dynamic, no?), but client/data/metadata all now load and seem to function properly on the client.

Not sure yet if I need to be casting to execute the core resources (open to advice there).

[avanderhoorn]

Looking good so far. I should mention that if we need to change some of the interfaces to allow this to work better, thats not too much of an issue. It would be ideal if you don't have to cast to the concrete types to get things to work, as people can potentially implement anything they want on the interfaces and we will want that to work.

[avanderhoorn]

In addition, not everything needs to be filtered through ClientScriptResource meaning if you want to leave RequestDataScript, ClientInterfaceScript and RequestMetadataScript going to their existing Resources, it could simplify the logic of the ClientScriptResource you are working on.

[MisterJames]

It would be ideal if you don't have to cast to the concrete types to get things to work...

Yeah, I agree. At this point I'm not sure there's a reason to, either, so I might do away with that.

And I'll consider bumping those core scripts back up a level, want to think through that a bit. I was working on a side method that executes the resource in interest of simplifying the whole thing...trying to balance between doing one thing and not needing it. I'll give'er another thunking tonight.

[avanderhoorn]

My thinking with leaving the core scripts out is that there are already resources that handle that and its a good separation of concern. But let us know how you go.

[MisterJames]

Okay...I think I'm going to take you up on that chance to hang out a bit on Skype or Hangouts or something (with screen share). I played around a bunch tonight, speaking into my earlier comment about the order/key-centric approach instead of the script-centric approach and I think I'd like some direction on which way to go. @avanderhoorn, would you have time an evening this week to tackle 30 mins or so of discussion? Also, noting that there are other, significantly higher-order priorities on the horizon, failing your availability perhaps @nikmd23 would be willing to entertain a Canadian for half an hour?

[avanderhoorn]

I'm happy to help out with this. I'll shoot your through an email with my skype on it.

[nikmd23]

More than willing to entertain you whenever @MisterJames.

If you see me on Skype (I'm almost always on) just ping me - I've got the same handle there.

[avanderhoorn]

@MisterJames Just wondering how you going with this one? Is there anything you have been waiting on us for this?

[MisterJames]

No...not waiting on anything that is blocking. I think you were going to send me a project that had an example of a client script, but that hasn't held me up. I believe I hacked through most of it, so I'll try to get it pushed up ASAP.