Replace wrench with node-fs-extra

[doapp-ryanp]

Fixes serverless/serverless#952

[doapp-ryanp]

Can you please update this plugin in npm? https://www.npmjs.com/package/serverless-optimizer-plugin still says v2.1.0 (not 2.1.1)

[HyperBrain]

According to the link I posted in the other task, the dereference option is only supported for the fs-extra.copy and not the fs-extra.copySync. I would suggest to use the asynchronous variant here, maybe wrapped by a Promise.fromCallback() to have a promisified implementation.
See also here: jprichardson/node-fs-extra#210

[doapp-ryanp]

@HyperBrain pretty sure its used in both. dereference is part of the stat method which is used by both:
https://github.com/jprichardson/node-fs-extra/blob/1cde1961f2e971cb42060384312c5faa419c9792/lib/copy/ncp.js#L50

EDIT: n/m looks like you are right. Will dig into this more and probably will end up replacing

[doapp-ryanp]

@HyperBrain hmm but jprichardson/node-fs-extra#210 indicates that copySync handles symlinks, which is prob why dereference opt only exists in async ver

[doapp-ryanp]

This small test confirms that copy and copySync work the same:

'use strict';

let fs = require('fs-extra');

fs.copySync('/tmp/tmp1', '/tmp/two', {clobber: true, dereference: true});

fs.copySync('/tmp/tmp1', '/tmp/three', {clobber: true, dereference: true}, function() {
    console.log('done');
});

/tmp/tmp1 $ ll
total 12K
drwxr-xr-x  5 ryan wheel  170 Apr  6 16:12 ./
drwxrwxrwt 50 root wheel 1.7K Apr  6 16:11 ../
-rw-r--r--  1 ryan wheel    3 Apr  6 16:02 hard.txt
lrwxr-xr-x  1 ryan wheel   28 Apr  6 16:07 me.jpg -> /Users/ryan/Dropbox/ryan.jpg
lrwxr-xr-x  1 ryan wheel   28 Apr  6 16:12 symDir -> /Users/ryan/Dropbox/branding/

/tmp/tmp1 $ ll /tmp/two/
total 12K
drwxr-xr-x  5 ryan wheel  170 Apr  6 16:12 ./
drwxrwxrwt 52 root wheel 1.8K Apr  6 16:12 ../
-rw-r--r--  1 ryan wheel    3 Apr  6 16:12 hard.txt
lrwxr-xr-x  1 ryan wheel   28 Apr  6 16:12 me.jpg -> /Users/ryan/Dropbox/ryan.jpg
lrwxr-xr-x  1 ryan wheel   28 Apr  6 16:12 symDir -> /Users/ryan/Dropbox/branding/

/tmp/tmp1 $ ll /tmp/three
total 12K
drwxr-xr-x  5 ryan wheel  170 Apr  6 16:12 ./
drwxrwxrwt 52 root wheel 1.8K Apr  6 16:12 ../
-rw-r--r--  1 ryan wheel    3 Apr  6 16:12 hard.txt
lrwxr-xr-x  1 ryan wheel   28 Apr  6 16:12 me.jpg -> /Users/ryan/Dropbox/ryan.jpg
lrwxr-xr-x  1 ryan wheel   28 Apr  6 16:12 symDir -> /Users/ryan/Dropbox/branding/

Or am I missing something?

Lastly, do you really want dereference? What if I have a config dir that has a prod.yml and dev.yml and a symlink that points config.yml at prod.yml. If you specify dereference you will get an error.

[HyperBrain]

is it a typo or did you execute fs.copySync() two times above?

[doapp-ryanp]

Nope you are totally correct. I executed copySync() 2x. After fixing output is as you expected. Async dereferences and sync does not.

My question still stands - why do you want to dereference? cp -R does not dereference and you can run into the problem in my example above.

[HyperBrain]

The biggest issue (that initiated the whole deref stuff in SLS) was, that some people have symlinked their node dependencies and it broke as long as the recursive copy did not dereference.
I assume that this approach (symlinking stuff esp. dirs) is quite common in build environments.

[doapp-ryanp]

My guess is they were doing relative symlinks vs full path ones like my example above. If they did full path it would have worked fine.

If I go the dereference route, there will be uses cases (rel links within same dir per my example above) that require a manual workaround.

So how about this, we go with the 80/20 rule thinking that a slim sub-set of people would symlink in the same dir. I'll re-implement with copy() and dereference. If someone does hit my use case, we just tell them sorry we don't support it and they need to manually copy the file vs symlink.

sound ok?

[HyperBrain]

yes, agree. I event doubt that it's 80/20 but more of a 90/10 :-)

[doapp-ryanp]

Addressed in #35