Skip to content

Upgrade date-fns@2 #8

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

Merged
merged 3 commits into from
Oct 3, 2019
Merged

Upgrade date-fns@2 #8

merged 3 commits into from
Oct 3, 2019

Conversation

bengry
Copy link
Contributor

@bengry bengry commented Sep 3, 2019

date-fns@2 has been released, and since listr-verbose-renderer is pinned on ^1.x.x, it makes it hard for users listr-verbose-renderer to use date-fns@2 simultaneously, due to conflicting packages.

This PR updates this dependency.

Note that this is a breaking change in listr-verbose-renderer for users who may have passed an invalid format (which previously worked) due to a breaking change in [email protected], to better conform the unicode standard

@bengry
Copy link
Contributor Author

bengry commented Sep 3, 2019

@SamVerschueren I'm not sure why the tests fail in Node 6. Can you please advise?

@SamVerschueren
Copy link
Owner

This is weird, according to the docs hook-std should be used differently 🤔 ...
https://github.com/sindresorhus/hook-std/tree/v1.1.0#usage

https://github.com/SamVerschueren/listr-verbose-renderer/blob/master/test/fixtures/utils.js

Looks like it should be something as this

'use strict';
const hookStd = require('hook-std');
const stripAnsi = require('strip-ansi');

exports.testOutput = (t, expected) => {
	t.plan(t._test.planCount || expected.length);
	let i = 0;

	const promise = hookStd(actual => {
		t.is(stripAnsi(actual), `${expected[i++]}`);

		if (i === expected.length) {
			promise.unhook();
		}
	});
};

@bengry
Copy link
Contributor Author

bengry commented Sep 5, 2019

@SamVerschueren what's the difference between what's currently in https://github.com/SamVerschueren/listr-verbose-renderer/blob/master/test/fixtures/utils.js and the snippet you posted in #8 (comment)?

They look the same to me.

Also, note that I didn't change the file in the first place. I'm not sure if master would even pass the above test with any minor changes to it (there's no lock file in the repo, which may be the cause of the discrepency)

@SamVerschueren
Copy link
Owner

The difference is the promise.unhook() instead of just unhook(). Not sure why that would only fail on Node.js 6 though...

@bengry
Copy link
Contributor Author

bengry commented Sep 5, 2019

@SamVerschueren I missed that in the diff. Thanks.
I pushed the change to this branch and seems like Travis is now happy in all checked versions.

Is this something that you want to merge to master and release a new version for? I mentioned the reasoning behind this PR in the OP.

@bengry bengry changed the title Upgrade date fns Upgrade date-fns to v2 Sep 7, 2019
@maraisr
Copy link

maraisr commented Sep 13, 2019

Mega keen on this merge!

@bengry
Copy link
Contributor Author

bengry commented Oct 1, 2019

@SamVerschueren any update on this?

@SamVerschueren SamVerschueren merged commit f3edfdf into SamVerschueren:master Oct 3, 2019
@SamVerschueren SamVerschueren changed the title Upgrade date-fns to v2 Upgrade date-fns@2 Oct 3, 2019
@SamVerschueren
Copy link
Owner

Sorry for the late replies and lack of response from my side. Released as 0.6.0.

@SamVerschueren
Copy link
Owner

Thanks for working on this! 🎉

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

Successfully merging this pull request may close these issues.

3 participants