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

sw-lib Namespacing #378

Closed
gauntface opened this issue Mar 22, 2017 · 8 comments
Closed

sw-lib Namespacing #378

gauntface opened this issue Mar 22, 2017 · 8 comments

Comments

@gauntface
Copy link

Library Affected:
sw-lib

Browser & Platform:
all browsers

Issue or Feature Request Description:
This was raised by @devnook

The current code organisation of sw-lib in terms of how everything is surfaced is unclear (i.e. swlib.cacheFirst() vs swlib.warmRuntimeCache() vs swlib.Route()

Does it make sense to move caching strategies to a higher level:

swlib.strategies.cacheFirst()

This frees up sw-lib methods to be at top level:

swlib.cacheRevisionedAssets(revisionedFiles)
swlib.warmRuntimeCache(revisionedFiles)

Should the exposure of Route be down via sw-lib, or should it be in it's original namespace?

@addyosmani
Copy link
Member

I'm strongly in favor of moving the caching strategies to their own namespace. I think keeping everything in the general namespace the way it is now lends itself to more confusion than we likely intend.

@addyosmani
Copy link
Member

Were there any other methods as part of this that we wanted to consider giving their own dedicated namespace?

@gauntface
Copy link
Author

Just what to do with swlib.Route which is a constructor for a custom Route instance.

I'm still fairly stumped on what to do with it to be honest. @jeffposnick any ideas?

@jeffposnick
Copy link
Contributor

I feel like most folks who interact with sw-lib won't instantiate Route directly, but will instead use the implicit route creation that they get from passing in a string or regex along with a handler. So moving swlib.Route out of the top-level namespace would not be problematic from my point of view.

@gauntface
Copy link
Author

Should I just remove Route altogether with the expectation that developers would need to pull in the extra module to make this work?

@jeffposnick
Copy link
Contributor

In principle, I'd be fine if you removed the basic Route class.

In the back of my mind, I worry about running afoul of #385, where the Router for the sw-lib module refused to accept instances of the Route class that come from the sw-routing module. That's probably not a reason to keep it around—I should just fix the underlying issue around class namespacing instead.

@addyosmani addyosmani added the P1 label Mar 31, 2017
@addyosmani
Copy link
Member

So moving swlib.Route out of the top-level namespace would not be problematic from my point of view.

👍

In principle, I'd be fine if you removed the basic Route class.

My reading is we're generally fine with removing the basic Route class, but need to address the issues around class namespacing to be less concerned with accidental breakage being caused. @jeffposnick would you like us to wait for you to fix issues in the later before moving forward here or is @gauntface safe to proceed?

@jeffposnick
Copy link
Contributor

I'd say proceed, as even if #385 can't be fixed easily, it's an edge case that developers aren't likely to bump into when using sw-lib.

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

3 participants