Skip to content
This repository has been archived by the owner on Jan 23, 2021. It is now read-only.

Revamp the names of confusing options #164

Closed
jeffposnick opened this issue Aug 31, 2016 · 10 comments
Closed

Revamp the names of confusing options #164

jeffposnick opened this issue Aug 31, 2016 · 10 comments

Comments

@jeffposnick
Copy link
Contributor

There are a few options whose names made sense to me initially, but are in retrospect not carefully named. I'd like to start by adding in aliases for those options, keeping the old ones functional but logging a deprecation message during build time. And then theoretically remove them in the next major revision. (Or keep them working, but remove them from the documentation?)

I was thinking of the following, and I'm open to suggestions:

  • staticFileGlobsfilesToCache
  • dynamicUrlToDependenciesserverRenderedUrls
  • navigateFallbackdefaultRoute
  • navigateFallbackWhitelistdefaultRouteWhitelist
  • replacePrefix, stripPrefix, stripPrefixMulti ⇒ try to wrap these up into a single modifyUrlPrefix option

CC: @addyosmani @gauntface @jpmedley for thoughts.

@jpmedley
Copy link
Contributor

I like all of these except for the navigateFallback and
navigateFallbgackWhitelist.

If I were completely new to the tool, I may not know what a route is. But I
can work out what navigateFallback probably is.

Joe Medley | Technical Writer, Chrome DevRel | jmedley@google.com |
816-678-7195
If an API's not documented it doesn't exist.

On Wed, Aug 31, 2016 at 7:46 AM, Jeffrey Posnick notifications@github.com
wrote:

There are a few options whose names made sense to me initially, but are in
retrospect not carefully named. I'd like to start by adding in aliases for
those options, keeping the old ones functional but logging a deprecation
message during build time. And then theoretically remove them in the next
major revision. (Or keep them working, but remove them from the
documentation?)

I was thinking of the following, and I'm open to suggestions:

  • staticFileGlobs ⇒ filesToCache
  • dynamicUrlToDependencies ⇒ serverRenderedUrls
  • navigateFallback ⇒ defaultRoute
  • navigateFallbackWhitelist ⇒ defaultRouteWhitelist
  • replacePrefix, stripPrefix, stripPrefixMulti ⇒ try to wrap these up
    into a single modifyUrlPrefix option

CC: @addyosmani https://github.com/addyosmani @gauntface
https://github.com/gauntface @jpmedley https://github.com/jpmedley
for thoughts.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#164, or mute the
thread
https://github.com/notifications/unsubscribe-auth/AH0vizl2YkfX-bZ37QYizoB0p1BD5LRwks5qlZOugaJpZM4JxsjG
.

@jpmedley
Copy link
Contributor

In other words, an option that explains itself is better than one I have to
read about.

Joe Medley | Technical Writer, Chrome DevRel | jmedley@google.com |
816-678-7195
If an API's not documented it doesn't exist.

On Wed, Aug 31, 2016 at 2:09 PM, Joe Medley jmedley@google.com wrote:

I like all of these except for the navigateFallback and
navigateFallbgackWhitelist.

If I were completely new to the tool, I may not know what a route is. But
I can work out what navigateFallback probably is.

Joe Medley | Technical Writer, Chrome DevRel | jmedley@google.com | 816-
678-7195
If an API's not documented it doesn't exist.

On Wed, Aug 31, 2016 at 7:46 AM, Jeffrey Posnick <notifications@github.com

wrote:

There are a few options whose names made sense to me initially, but are
in retrospect not carefully named. I'd like to start by adding in aliases
for those options, keeping the old ones functional but logging a
deprecation message during build time. And then theoretically remove them
in the next major revision. (Or keep them working, but remove them from the
documentation?)

I was thinking of the following, and I'm open to suggestions:

  • staticFileGlobs ⇒ filesToCache
  • dynamicUrlToDependencies ⇒ serverRenderedUrls
  • navigateFallback ⇒ defaultRoute
  • navigateFallbackWhitelist ⇒ defaultRouteWhitelist
  • replacePrefix, stripPrefix, stripPrefixMulti ⇒ try to wrap these up
    into a single modifyUrlPrefix option

CC: @addyosmani https://github.com/addyosmani @gauntface
https://github.com/gauntface @jpmedley https://github.com/jpmedley
for thoughts.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#164, or mute the
thread
https://github.com/notifications/unsubscribe-auth/AH0vizl2YkfX-bZ37QYizoB0p1BD5LRwks5qlZOugaJpZM4JxsjG
.

@jeffposnick
Copy link
Contributor Author

Thanks, @jpmedley. I can leave navigateFallback as-is.

Longer-term, I want to address routing to multiple App Shells via #94, at which point including routing in a parameter name might make sense.

@jasan-s
Copy link

jasan-s commented Sep 3, 2016

I like the change in name.

@addyosmani
Copy link
Contributor

I like the renames. I'm personally also fine with:

  • navigateFallbackdefaultRoute
  • navigateFallbackWhitelistdefaultRouteWhitelist

But if consensus is a first time user is going to find it straight-forward to understand what these do as-is then let's for sure keep them as is.

@gauntface
Copy link

gauntface commented Mar 6, 2017

Probably just me but I'd understand what is intended with 'serverRenderedUrls' if it included the term 'template' somewhere in the name.

'templatesToUrls' would make sense to me more immediately than 'serverRenderedUrls'.

All others are big +1 from me.

@jeffposnick
Copy link
Contributor Author

templatesToUrls sounds like a mapping in which each key is a template, and the values are one or more URLs. That doesn't reflect what's being configured here.

Including "template" in the name sounds good, so how about templatedUrls?

While I normally use abcToXYZ to name variables that represent mappings, that's not a hard and fast convention.

@gauntface
Copy link

@jeffposnick sounds better than my suggestion 👍

@addyosmani
Copy link
Contributor

Echoing further support for templatedUrls. I think that's a lot more sensible a name to move forward on.

@jeffposnick
Copy link
Contributor Author

We've switched to improved names for a number of similar parameters in https://workboxjs.org/

Making a breaking change to rename the options in sw-precache isn't a priority.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants