-
Notifications
You must be signed in to change notification settings - Fork 72
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
Depcache spec and reference implementation #210
base: main
Are you sure you want to change the base?
Conversation
@guybedford, thanks! We will need you to join the WICG to merge this. You should have an invite in your inbox. Let me know once that is done, and then we can merge this. |
Brainstorming some issues (not hard, so I expect we can figure out solutions later; right now I don't have bandwidth though): I prefer not to call
Also, I suppose the current PR uses |
@marcoscaceres thanks for the info here. I should already be a member I believe? I was sent an email to link my GitHub profile, which I did after posting this PR, but the check hasn't seemed to have updated. Any tips on further steps I can take here welcome. |
I think this might be best in a separate repository, so it can gather its own community and interested implementers. |
@domenic I'm a little weary of aiming to get interest for a spec on top of a spec on top of a spec. The calls from such a depcache spec to WICG import maps will be quite hand-wavey with some complex interactions. In addition I did need to perform some injection and refactoring to the existing import maps algorithms. If you think this is the best approach, then I think a fork might be preferable, with the intent to re-merge. I would just like to see SystemJS and ES module shims built to spec work here on the optimization problems which is my primary interest as an implementor, so this seems like the right sort of route to me. Would you be ok with that? |
I mean, I have no objection to you forking. I just think it's important to separate the import maps community in this repository from any additional proposals, which have different levels of support. I'm OK with keeping one issue open (e.g. either #208 or #209) to direct folks from this community into that separate body of work, but I don't think it's a good idea to accept proposals like this into the import maps spec without (browser) implementer interest. |
@guybedford thanks! The IPR-checking system has found you :) all green. |
@domenic is there any way you could still be convinced this is a useful feature for this group to consider? Have you followed the issue discussion? I just haven't really heard a real argument from you for why you don't think this feature is suitable for this spec. Personally I see this as a critical feature for production modules, and first opened an issue on this problem two years ago. I'm happy to assist with spec work, implementation work and even creating a Chromium CL for it. |
I'm just looking to finish import maps. I am sure there are a ton of useful features in the modules space, of varying criticality in different peoples' judgments. I think they need to build their own communities and support. (I have moved on from working on modules, so import maps will be my last project in that space.) |
@hiroshige-g I'm sympathetic with the aim of simplifying/unifying recursion and working with cycles (it's complicated on the JS side too!), but it looks like if this preloading happened during "fetch the descendants of a module script", it would happen after the fetch has actually completed and we have the whole module, which seems unnecessarily delayed. Was that the intention? |
Ah, ok! if import-maps is mostly "done" per #210 (comment) - and more or less ready to migrate out of the WICG, then yes, I agree with @domenic forking would be appropriate here. |
This PR implements the
"depcache"
proposal in #209, providing an optimal solution to the waterfall problem, while ensuring that import maps remain the source of truth for the graph and optimization for tools that generate them.Let's keep proposal feedback to the issue, and direct spec feedback to this PR.
The approach I've implemented here is to effectively integrate the depcache check into
fetch a single module script
in the HTML spec, where the recursion between this algorithm callingpreload depcache
, which in turn is callingfetch a single module script
perform the graph traversal. This traversal is very simple and naturally stops on unnecessary iterations due to the module map existence checks, avoiding the need for visited lists and such.To support the new
resolve a module specifier
signature, I've separated out the top-level call to it from HTML from the base-level API-like call to its resolution, calledresolve import maps
, which is just takes a specifier, import map and baseURL. This way the depcache preload can easily call into that resolution. Error handling has been carefully specified as well here.Any changes to the
fetch a single module script
signature in further refactoring can be applied equally to the depcache preload algorithm, although it is worth noting that the script for depcache preloads doesn't exist at the time of the network request.Feedback welcome, happy to keep working on this.
Preview | Diff