mischnic / parcel-plugin-sw-cache

📦👷 Parcel plugin for caching using a service worker
https://npm.im/parcel-plugin-sw-cache
MIT License
47 stars 6 forks source link

allowing relative paths for dest. fixing #10 #11

Closed dsych closed 6 years ago

dsych commented 6 years ago

As described in #10, rebuild is failing because old config modification are retained which means that swDest path gets appended to itself.

Additional feature:

mischnic commented 6 years ago

Would const config = Object.assign({}, bundle.entryAsset.package.cache); fix it as well? Would useRelativeDest still be needed then?

Allowing user to specify whether they want to use relative path for the swDest, since it will allow injection into the same file that is pointed by swSrc. This is useful there is an existing service worker that needs to be injected with precache routes, but it can't be generated after bundling is done, since source now contains all of the imports required by the SW.

I don't quite understand what you want to achieve. Workbox injection works by literally replacing workboxSW.precache([]); in swSrc with workboxSW.precache([{/* [cache entries] */ }]);. So overwriting the source renders it useless for following builds.

dsych commented 6 years ago

const config = Object.assign({}, bundle.entryAsset.package.cache); would also fix the problem, however, I would still leave the resolve part, since you don't want to limit the destination folder to only be under the dist of parcel. You should still be able to specify things like ../othe-dir/my-sw.js.

Concerning the useRelativeDest, you would use it on the bundeled version of the sw, not the source. As I mentioned before, there could be dependencies inside sw that you would like to retain. However, the way I am using it here is just by leveraging a bug inside workbox-build, which is not something you want to see in a stable package. So what I did instead is file a feature request on workbox.

mischnic commented 6 years ago

I would still leave the resolve part, since you don't want to limit the destination folder to only be under the dist of parcel.

There is no difference to join:

$ node
> const outDir="/user/parcel-plugin-sw-cache/example/dist"
> const swDest="../dir/sw.js"
> require("path").join(outDir, swDest)
'/user/parcel-plugin-sw-cache/example/dir/sw.js'
> require("path").resolve(outDir, swDest)
'/user/parcel-plugin-sw-cache/example/dir/sw.js'
dsych commented 6 years ago

They are not the same if your are feeding absolute paths.

$ node
> const outDir = "/tmp/out.txt"
> const swDest = "/tmp/out2.txt"
> const path = require("path")
> path.join(outDir, swDest)
'\\tmp\\out.txt\\tmp\\out2.txt'
> path.resolve(outDir, swDest)
'\\tmp\\out2.txt'
mischnic commented 6 years ago

They are not the same if your are feeding absolute paths.

And why would you use an absolute swDest?

dsych commented 6 years ago

I guess you have a point, if we use const config = Object.assign({}, bundle.entryAsset.package.cache);, the join should be fine. The paths are absolute if you don't do a deep copy of the config object, since it gets carried over between re-builds.

mischnic commented 6 years ago

I guess you have a point, if we use const config = Object.assign({}, bundle.entryAsset.package.cache);, the join should be fine.

😄

Could you change that back?

dsych commented 6 years ago

Done.

mischnic commented 6 years ago

Thanks! Published as 0.2.5