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

add option for each stripPrefix to have own replacePrefix #142

Closed

Conversation

cdbattags
Copy link

depends on #138 from @SignpostMarv

Building off of #138, I wanted to add functionality for each each stripPrefix to have it's own replacePrefix. The use case would be having multiple "modules" that each have their own "public" directory that are all globbed into the sw.js file.

Format for stripPrefix if you'd like the functionality is as follows:

{
  stripPrefix: [
    [strip1, replace1],
    [strip2, replace2]
  ]
}

or

{
  stripPrefix: [
    {
      strip: stripString1,
      replace: replaceString1
    },
    {
      strip: stripString2,
      replace: replaceString2
    }
  ]
}

SignpostMarv and others added 2 commits June 29, 2016 23:02
…ths to another directory (i.e. server sends foo/file.txt for requests to bar/file.txt, as well as caching bar/other-file.txt)
@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm.

@cdbattags
Copy link
Author

Hey @SignpostMarv! Please confirm you're ok with the change and maybe we can get these both in at once.

Also, any suggestions?

@SignpostMarv
Copy link
Contributor

Thinking something like this:

var stripPrefixAsObj = {
  "replaceThis": "withThat",
  "replaceThisOtherThing": "withThatOtherThing"
};
var relativeUrl = fileAndSizeAndHash.file.replace(
  new RegExp('/^(' + Object.keys(stripPrefixAsObj).map(escapeRegExp).join('|') + ')'),
  function (match) {
    return stripPrefixAsObj[match];
  }
);

@cdbattags
Copy link
Author

Nice! I like it. I'll update my PR in a few.

@jeffposnick jeffposnick self-assigned this Jul 11, 2016
@jeffposnick
Copy link
Contributor

Sorry, I've been offline for a bit and am catching up on things at the moment.

Just so I understand the scope of what's in this PR, it seems like it includes everything from @SignpostMarv's #138, correct? So I can close that PR and just provide feedback on this?

@SignpostMarv
Copy link
Contributor

SignpostMarv commented Jul 11, 2016

@jeffposnick I think we're still waiting on @cdbattags to update it with the Object.keys version, although I've got time tonight to patch it as well.
@jeffposnick @cdbattags I've updated #138 to include the mapped prefix-replace object method described above.

@jeffposnick
Copy link
Contributor

Thanks for the original contribution, @cdbattags.

I've merged #138, and if you want to reconcile the conflicts in your current PR and ensure all linting/tests pass, I'll be happy to merge yours.

@@ -0,0 +1,3 @@
[*]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer not to check this in.

@cdbattags
Copy link
Author

Hi all! Sorry for the delay! Yeah, @jeffposnick, with the .editorconfig file I just wanted to put that out there and get a feel for why most repos are against checking one in 😳. Also, it looks like @SignpostMarv's #138 covers what I implemented, no?

@jeffposnick
Copy link
Contributor

Okay, if you're happy with the implementation in #138 and feel like it covers the same ground, I'm happy to close this and will tag a new 4.0.0 release shortly. Thanks for the original contribution and for collaborating with @SignpostMarv.

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

Successfully merging this pull request may close these issues.

4 participants