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

Support dates #18

Open
sindresorhus opened this issue Jun 18, 2017 · 9 comments

Comments

@sindresorhus
Copy link
Owner

commented Jun 18, 2017

Issuehunt badges

Would be nice if you could store.set('foo', new Date()) and get back a Date object when you do store.get('foo'). Could probably make use of the second argument to JSON.parse(). We need to save some kind of information to indicate that it's a native Date format. Maybe a separate __meta__ key or something. Suggestions welcome.


IssueHunt Summary

Backers (Total: $80.00)

Submitted pull Requests


Become a backer now!

Or submit a pull request to get the deposits!

Tips


IssueHunt has been backed by the following sponsors. Become a sponsor

@sholladay

This comment has been minimized.

Copy link

commented Jul 3, 2017

You could serialize dates to ISO 8601 format in .set(). Then you could parse ISO 8601 dates into Date objects in .get() but override their .toString() to return the string as it was stored (rather than the default (new Date()).toString() funkiness).

That would:

  • Avoid adding extra fields
  • Be convenient, regardless of how the date was set (either as a string literal or a Date object)
  • Be safe, in case the date was set as a string literal
@sindresorhus

This comment has been minimized.

Copy link
Owner Author

commented Jul 3, 2017

Then you could parse ISO 8601 dates into Date objects in .get() but override their .toString() to return the string as it was stored (rather than the default (new Date()).toString() funkiness).

That sounds hackish. What if the user tries to do typeof store.get('date-string') and expected it to be string, not Date. I would rather store some extra metadata than having unexpected behavior.

@acheronfail

This comment has been minimized.

Copy link

commented Jan 27, 2018

Just another suggestion:

Rather than a separate __meta__ key, what are your thoughts on prepending the date type to the value? Meaning:

const d = new Date();
const s = d.toString();

// Current:
conf.set('date', d);    // { "date": "Sat Jan 27 2018 18:42:32 GMT+1100 (AEDT)" }
conf.set('dateStr', s); // { "date": "Sat Jan 27 2018 18:42:32 GMT+1100 (AEDT)" }

// Suggested:
conf.set('date', d);    // { "date": "__date__,Sat Jan 27 2018 18:42:32 GMT+1100 (AEDT)" }
conf.set('dateStr', s); // { "date": "Sat Jan 27 2018 18:42:32 GMT+1100 (AEDT)" }

If the value passed to .set() is an instance of Date we could prepend a special string to the serialised string, and just check for that when parsing it back from JSON.

@sindresorhus

This comment has been minimized.

Copy link
Owner Author

commented Feb 5, 2018

@acheronfail Can't do that. What if the user actually sets __date__,Sat Jan 27 2018 18:42:32 GMT+1100 (AEDT) themselves and expects a string back.

@sindresorhus

This comment has been minimized.

Copy link
Owner Author

commented Sep 9, 2019

conf (which is where this should be first implemented) now has a __internal__ that we could use to store this kind of metadata.

@issuehunt-app

This comment has been minimized.

Copy link

commented Sep 9, 2019

@issuehunt has funded $80.00 to this issue.


@rafaelramalho19

This comment has been minimized.

Copy link
Contributor

commented Sep 9, 2019

How about adding the possibility to store a function in the store and execute it automatically?

e.g. conf.set('foo', () => new Date());

And whenever the user fetches the value, we just evaluate if it's a function or not and run it.

@sindresorhus

This comment has been minimized.

Copy link
Owner Author

commented Sep 9, 2019

@rafaelramalho19 I don't think that's a good idea. That would be a security nightmare. Anything on the computer could add code to the plaintext config file and have the app execute it in a trusted context.

@rafaelramalho19

This comment has been minimized.

Copy link
Contributor

commented Sep 9, 2019

I guess thats true. Just wanted it to allow for broader use-cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.