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

Proposal: pass custom params in ServiceWorkerRegistration for future use #1157

Closed
laukstein opened this issue May 31, 2017 · 15 comments
Closed

Comments

@laukstein
Copy link

Current spec https://w3c.github.io/ServiceWorker/#navigator-service-worker-register

I want be able to pass custom parameters to ServiceWorker for future use, since localStorage and sessionStorage aren't allowed. It would be grate be able to defined version, URLs to cache, etc. early in navigator.serviceWorker.register like:

navigator.serviceWorker.register("sw.js", {
    scope: "/",
    customParamFoo: "test",
    bar: {
        version: "1.2.0",
        environment: "development"
    },
    urlsToCache: ["style.css", "icon.png"]
}

and after in sw.js use like:

self.registration.customParamFoo;
const version = self.registration.bar.version;
const urlsToCache = self.registration.urlsToCache;

Any early proposal for it? Any reason why not to implement this?

@asutherland
Copy link

Your example parameters seem like data that should be inlined into the body of your sw.js script or its dependencies (once #839 is implemented in all browsers so that changes to dependencies can trigger updates). This avoids introducing a de facto new storage API and provides clear semantics around ServiceWorker updates. (It's also a win/win from a browser engine and SW author perspective because there's no extra moving parts that could delay SW startup.)

Note that your script URL can be something like sw.js?foo=bar&baz=foo with your server inlining the parameters into your script.

@laukstein
Copy link
Author

@asutherland, I prefer not to change a file name, it will cause additional download for sw.js because of query URL. And when used hash URL like sw.js#foo=bar&baz=f, the hash isn't passed to location.pathname... what makes the only way yo pass params is by query URL.
Also, in order to avoid duplicate content, I trim queries from URLs by server-side, and it makes this issue still relevant.

@asutherland
Copy link

@laukstein Can you elaborate on your use-case, particularly in how it would work in terms of updates? It's not clear to me whether you want to use the parameters to avoid having to include, for example, https://github.com/GoogleChrome/sw-toolbox configuration in the SW script itself. Or if the parameter would be something higher level, such as picking which branch/release of an app to use (such as is discussed in #1085).

Also of relevance is how you would plan to detect and correct ServiceWorkers stuck with a bad parameter configuration.

Aside: If you're concerned about additional downloads of sw.js, you may also want to consider using useCache=true and ensuring your server has appropriate cache settings, because SW's really like to do update checks. If things aren't well-cached then the script and all its dependencies could be re-downloaded so that the byte-wise comparison check can be performed.

@laukstein
Copy link
Author

laukstein commented Jun 5, 2017

@asutherland my question isn't related spec issue, and not #1085. I want to pass whatever params to sw.js without adding params in URL. Imagine like HTML having multiple JS files with global variables - each of the files can after read those global variables, while in sw.js it is not allowed, even will not access localStorage ... I want be able to define whatever params in navigator.serviceWorker.register that after would be used in sw.js. In top comment you can see few usable cases for it (version, environment, urlsToCache).

@jakearchibald
Copy link
Contributor

Why isn't indexeddb the solution here?

@laukstein
Copy link
Author

@jakearchibald serviceWorker works also in private browsing (incognito window), while IndexedDB doesn't not in Firefox (similar with localStorage in Safari). Another issue, I wouldn't want serviceWorker to wait for async IndexedDB response to get version, urlsToCache, etc. it would impact serviceWorker response time.

@jakearchibald
Copy link
Contributor

jakearchibald commented Jun 6, 2017

@laukstein

serviceWorker works also in private browsing (incognito window), while IndexedDB doesn't not in Firefox (similar with localStorage in Safari)

This is a common mistake in standards discussion (and don't worry about it, it's a mistake I've made many times before). You cannot compare the current state of things, beholden to reality, with a utopian implementation of some currently non-existent thing.

While IndexedDB may not currently work in Firefox when in incognito mode, your customParamFoo proposal doesn't currently work in any browser, incognito or not.

I wouldn't want serviceWorker to wait for async IndexedDB response to get version, urlsToCache, etc. it would impact serviceWorker response time.

Do you have data on the performance issue here? It could be something that's fixable. If looking up data is inherently slow, your customParamFoo proposal will take that hit every time the service worker starts up, whether the developer needs that data or not.

@laukstein
Copy link
Author

laukstein commented Jun 6, 2017

@jakearchibald why not simply allow pass custom params (whatever) like scope to ServiceWorker trough navigator.serviceWorker.register as mentioned in top comment? Is it about security?

Imagine array with urlsToCache is generated with JavaScript before navigator.serviceWorker.register ...
today there are no way to get it work without impact on performance, like storing in IndexedDB or place in sw.js?query-url-with-all-urlsToCache...

In my case I am stuck with urlsToCache.

@jakearchibald
Copy link
Contributor

@laukstein

why not

The addition of new features needs to be justified. It's more "why" than "why not" (see burden of proof).

I haven't considered security issues with your proposal, but there's no point in adding it if we already have a feature that achieves roughly the same thing, and more.

Imagine array with urlsToCache is generated with JavaScript before navigator.serviceWorker.register ...
today there are no way to get it work without impact on performance, like storing in IndexedDB

Are you sure this is bad for performance? If you're using this as part of an installing worker it isn't blocking anything, and any performance penalty is surely minuscule compared to the network activity.

or place in sw.js?query-url-with-all-urlsToCache...

Can you explain why this would perform worse than your proposal?

@laukstein
Copy link
Author

@jakearchibald

like storing in IndexedDB

it will require addition async call before get to ServiceWorker events.

sw.js?query-url-with-all-urlsToCache...

  • I trim queries from URLs by server-side to avoid duplicate content, and cache/serve files without queries.
  • The long query can exceed the limit supported by server and browser.
  • Array as string in URL after will be needed to be converted to array, what if passing Date in URL, multiple level Object (JSON) as string, etc.

All this could be avoided with simple support by spec as proposed above.

@jakearchibald
Copy link
Contributor

jakearchibald commented Jun 6, 2017

@laukstein you need to compare like-for-like here. In your proposal the browser is doing the work for you, but that doesn't mean it takes 0ms.

[indexeddb] will require addition async call before get to ServiceWorker events.

Right, but in your proposal self.registration.customParamFoo needs to be provided synchronously, whether the developer wants it or not. That doesn't seem better, it recreates the problems of localstorage.

I trim queries from URLs by server-side to avoid duplicate content, and cache/serve files without queries

This isn't a browser problem, it's a problem you're creating yourself.

The long query can exceed the limit supported by server and browser.

Whooaaaa how much data are you needing to pass in? If it's that much, the synchronous deserialising of that data is likely to be slooowww.

Array as string in URL after will be needed to be converted to array, what if passing Date in URL, multiple level Object (JSON) as string, etc.

Right, but in your solution the data needs to be structured-clonable and deserialised per service worker startup. Just because the browser is doing it for you, doesn't make it free.

For comparison's sake, here's what I'm proposing (using idb-keyval), which works today:

In your page:

async function registerSw() {
  await idbKeyval.set('urlsToCache', ['/', '/cat.gif']);
  return navigator.serviceWorker.register('/sw.js');
}

In your service worker:

addEventListener('install', event => {
  event.waitUntil(async function() {
    const [cache, urls] = await Promise.all([
      caches.open('static-v1'),
      idbKeyval.get('urlsToCache')
    ]);
    await cache.addAll(urls);
  }());
});

@wanderview
Copy link
Member

While IndexedDB may not currently work in Firefox when in incognito mode, your customParamFoo proposal doesn't currently work in any browser, incognito or not.

FWIW, service workers also don't work in private browsing mode in Firefox.

@laukstein
Copy link
Author

Is there any spec - if anything expected to differ on private browsing? Or all that considered as browsers bugs and lack of support?

@wanderview
Copy link
Member

I don't think private browser stuff is spec'd. We have some work in progress to make IDB work in private browsing mode. Its unclear if we will enable service workers there, though.

@jakearchibald
Copy link
Contributor

Closing this.

I wish we could just spec async localstorage, but I guess once you add something to handle concurrency issues, you're right back at IDB.

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

4 participants