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

conn.newer doesn't appear to work with Node v8+ #90

Closed
JacobDB opened this issue Jun 20, 2017 · 12 comments
Closed

conn.newer doesn't appear to work with Node v8+ #90

JacobDB opened this issue Jun 20, 2017 · 12 comments

Comments

@JacobDB
Copy link

JacobDB commented Jun 20, 2017

I have a Gulp task that uploads changed files, but since installing Node v8 (specifically tested with v8.1.2), it doesn't appear to be working correctly anymore; New files get added just fine, but old files never get replaced, unless I first delete them off the server, or remove the "newer" check.

I've just tested with an older version of Node (v6.10.3), and the task worked exactly as expected.

JacobDB/new-site/gulp-tasks/upload.js

@moequraishi
Copy link

Same issue, this needs to be fixed asap.

@JacobDB
Copy link
Author

JacobDB commented Jun 23, 2017

Thanks, I thought I was going crazy for a while lol

@JacobDB
Copy link
Author

JacobDB commented Jun 26, 2017

After some testing, it appears to be a change in the way the times are retrieved with remote.ftp.date at /lib/filter.js:23. My server's time is being reported in GMT, while my local time is CDT (GMT-5). Setting the timeOffset to -300 fixes the issue, but I didn't have to do this previously, so I'm not sure what changed.

Adding console.log(ftp.stat.mtime); and console.log(remote.ftp.date) to the newer function, I get the following reported:

file.stat.mtime: Mon Jun 26 2017 10:49:00 GMT-0500 (CDT)
remote.ftp.date: Mon Jun 26 2017 15:49:09 GMT-0500 (CDT)

As you can see, it thinks the remote time is in CDT even though it's actually being retrieved in GMT.

@JacobDB
Copy link
Author

JacobDB commented Aug 14, 2017

@morris I just tried looking in to this again, no luck. It seems like the local time is reporting accurately, but the remote time isn't. Can't figure out why. Any tips on where to look?

@JacobDB
Copy link
Author

JacobDB commented Sep 29, 2017

Keep coming back to this with no success. No matter what I try, I can't get the server time to be reported correctly. I guess I'll just have to resort to manually specifying the offset for now.

@JacobDB
Copy link
Author

JacobDB commented Nov 1, 2017

Now that Node v8 has moved in to LTS, it'd be awesome if this could be fixed.

@nandaleite
Copy link

I have noticed this problem recently (Newest files not being uploaded).
Thanks for pointing out the remote.ftp.date issue.

Looks like Nodejs is converting the date to UTC automatically.

I am using this code as a workaround:

          if (modify) {
            var year = modify.substr( 0, 4 );
            var month = modify.substr( 4, 2 );
            var date = modify.substr( 6, 2 );
            var hour = modify.substr( 8, 2 );
            var minute = modify.substr( 10, 2 );
            var second = modify.substr( 12, 2 );
            var date = new Date(year + '-' + month + '-' + date + 'T' + hour + ':' +minute + ':' + second);
            var timezone = date.getTimezoneOffset() / 60;
            obj.date = date.setHours(date.getHours() - timezone);

          }

At mlsd.js

vinyl-ftp/lib/mlsd.js

Lines 91 to 100 in 10ba868

if (modify) {
var year = modify.substr( 0, 4 );
var month = modify.substr( 4, 2 );
var date = modify.substr( 6, 2 );
var hour = modify.substr( 8, 2 );
var minute = modify.substr( 10, 2 );
var second = modify.substr( 12, 2 );
obj.date = new Date(
year + '-' + month + '-' + date + 'T' + hour + ':' +minute + ':' + second
);

@jacobraccuia
Copy link

nandaleite's solution worked for me, thank you! running npm v 5.6.0, node v.8.9.4

@selrond
Copy link

selrond commented Mar 28, 2018

Any news on this?

@jpacareu
Copy link

Well I don't know if this is a proper solution but I had to install Node Version Manager (for windows go here and install the latest stable version I mean the one with the green label next to it). Once this is done open the CMD and run
nvm install 7.10.1
Wait for the installation to finish and type:
nvm use 7.10.1
and finally run your gulp command in your project. Hope it helps .... Cheers :)

@selrond
Copy link

selrond commented Apr 15, 2018

@jpacareu the title of this issue explicitly mentiones vinyl-ftp not working with "Node v8+"; you've installed node 7.10.1, so it's out of the scope of this issue.

@JacobDB
Copy link
Author

JacobDB commented Jan 12, 2024

Commented earlier and deleted because I was wrong – I've now figured it out.

This started occurring again after I switched from CommonJS syntax to ESM syntax. No idea why, I'm honestly not familiar enough with the difference to understand, I'm just now learning about ESM. It appears that Node.js's file.stat.mtime simply doesn't contain timezone information for modified files, so vinly-ftp has no idea that the timezone is off. Instead, vinly-ftp offers an timeOffset parameter, which you can feed in the number of minutes you need to offset. This isn't ideal, as in places with DST, that time offset will change twice a year.

So, you can work around this by dynamically setting the timeOffset based on your local machine's timezone, like so:

/**
 * Ensure timezone offset is set
 */
if (global.settings.ftp.timeOffset === null || global.settings.ftp.timeOffset === undefined) {
    const DATE = new Date();
    global.settings.ftp.timeOffset = DATE.getTimezoneOffset() * -1;
}

This assumes that your server is set to GMT time, your configuration object is set to global.settings.ftp, and that timeOffset hasn't been set, or is set to null. So far, this seems to be working perfectly, but I've only tested it in my own timezone (GMT-6), so I'm not sure how it'll work elsewhere in the world; in theory, it should be fine but the * -1 has me concerned for some reason. In any case, this should be a good enough starting point to figure it out if your environment differs than mine.

Super frustrating issue, but I think this is a good enough solution for me, at least for now.

@JacobDB JacobDB closed this as completed Jan 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants