Skip to content
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

Tagged source #17

Open
fabiancook opened this issue Dec 8, 2023 · 3 comments
Open

Tagged source #17

fabiancook opened this issue Dec 8, 2023 · 3 comments

Comments

@fabiancook
Copy link

fabiancook commented Dec 8, 2023

This is related to full picture, not to current state

A callback style source was suggested here: comment link

With a rebuttal that it wouldn't work with a function reference as the install event scope may no longer be accessible when the route is used: comment link

The reasoning for not having a function reference makes sense as a source, but being able to use either a tag to shortcut re-routing inside the fetch handler, or providing a function name to use as a name for an exported function from a service worker would be helpful.

Tag functionality would be similar to the sync API.

I've also included an alternative where a function source type could be added, but I feel like its over reaching compared to tagging.

Example

addEventListener("install", event => {
 event.addRoutes([
   {
     condition: [
       { requestMethod: "get" },
       { urlPattern: { pathname: "/api/users" }
     ],
     source: [
       {
         type: "cache"
       },
       {
         type: "fetch-event",
         // Addition to the fetch-event or race-network-and-fetch-handler sources
         tag: "listUsers"
       }
     ]
   },
   {
     condition: [
       { requestMethod: "post" },
       { urlPattern: { pathname: "/api/users" }
     ],
     source: [
       {
         type: "fetch-event",
         tag: "createUser"
       }
     ]
   }
 ])
})

const handlers = {
  listUsers,
  createUser
}

addEventListener("fetch", event => {
  if (event.tag === "listUsers") {
    event.respondWith(listUsers(event));
  } else if (event.tag === "createUser") {
    event.respondWith(createUser(event));
  }
});
Alternatives

Named Export

addEventListener("install", event => {
 event.addRoutes([
   {
     condition: [
       { requestMethod: "get" },
       { urlPattern: { pathname: "/api/users" }
     ],
     source: [
       {
         type: "cache"
       },
       {
         type: "function",
         name: "listUsers"
       }
     ]
   },
   {
     condition: [
       { requestMethod: "post" },
       { urlPattern: { pathname: "/api/users" }
     ],
     source: [
       {
         type: "function",
         name: "createUser"
       }
     ]
   }
 ])
})

export {
  listUsers,
  createUser
}

Mixed Functionality

addEventListener("install", event => {
 event.addRoutes([
   {
     condition: [
       { requestMethod: "get" },
       { urlPattern: { pathname: "/api/users" }
     ],
     source: [
       {
         type: "cache"
       },
       {
         type: "function",
         name: "fetch",
         tag: "listUsers"
       }
     ]
   },
   {
     condition: [
       { requestMethod: "post" },
       { urlPattern: { pathname: "/api/users" }
     ],
     source: [
       {
         type: "function",
         name: "fetch",
         tag: "createUser"
       }
     ]
   }
 ])
})

export function fetch(event) {
  if (event.tag === "listUsers") {
    return listUsers(event);
  } else if (event.tag === "createUser") {
    return createUser(event);
  }
}
fabiancook added a commit to virtualstate/internal that referenced this issue Dec 8, 2023
@yoshisatoyanagisawa
Copy link
Collaborator

Thank you for the proposal, and sorry for the slow reply.
As far as I understand from the use case of the tag you proposed, I feel its use case similar to RouterSourceFetchEvent id in Jake's original proposal. Id in RouterFetchEventSource in the final form is based on that.

Can I ask you to tell more on how the tag is different from the id? Especially, what is a use case that the tag can cover, but the id cannot?

@fabiancook
Copy link
Author

fabiancook commented Jan 16, 2024

Ah! I had missed id and routerCallbackId, this is what I was after.

And I note that I see the fetch event has many <type>Id fields, which this would be inline with.

I would mention though routerCallback seems a bit long winded for this specific property, where it doesn't seem like this is really a callback but it is a fetch handler.

routeId seems like it would be a shorter nicer name for this property. Even routeSourceId, as its more to do with the source than the callback (It may be many callbacks intercepting this)

This issue is resolved though with this id in the options at least! I would just be curious as to if routerCallbackId would be the final property name now that the registerRouter naming has been dropped in favour of addRoutes

@yoshisatoyanagisawa
Copy link
Collaborator

A name for id (both in used as a router rule's "fetch-event" attribute and a field showing up in the fetch event) has not been decided yet. Since a router rule also has its own ID to be used for identifying the rule in Chromium devtools, I am hesitate to use routeId. At the same time, I understand a short name is easy to write. Let me listen what you and other folks think on this.

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

No branches or pull requests

2 participants