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

Timestamp #61

Closed
lookfirst opened this issue Aug 4, 2019 · 7 comments
Closed

Timestamp #61

lookfirst opened this issue Aug 4, 2019 · 7 comments

Comments

@lookfirst
Copy link
Contributor

I'm working with Firestore which has a Timestamp class. I ended up using that instead of putting Date.toISOString() everywhere.

Not sure if you want to add dependencies on outside projects so I'm doing this as an issue first vs. a PR ... here is my code...

import {Kind} from 'graphql/language';
import {GraphQLScalarType} from 'graphql';
import * as admin from "firebase-admin";
import Timestamp = admin.firestore.Timestamp;

export const TimestampScalar = new GraphQLScalarType({
	name: 'Timestamp',
	serialize(date: Timestamp) {
		return date.toDate().toJSON();
	},
	parseValue(value) {
		const date = new Date(value);
		// eslint-disable-next-line no-restricted-globals
		if (Number.isNaN(date.getTime())) {
			throw new TypeError(`Value is not a valid Date: ${JSON.stringify(value)}`);
		}
		return Timestamp.fromDate(date);
	},
	parseLiteral(ast) {
		if (ast.kind === Kind.INT) {
			return Timestmap.fromDate(new Date(parseInt(ast.value, 10)));
		} else if (ast.kind === Kind.STRING) {
			if (this.parseValue)
				return this.parseValue(ast.value);
		}
		return null;
	},
});
@ardatan
Copy link
Collaborator

ardatan commented Aug 5, 2019

We already have DateTime which applies most cases for timestamps, and DateTime accepts native Date instance, numbers and strings in a proper format. This looks firebase-specific in my opinion. What do you think? @Urigo

@lookfirst
Copy link
Contributor Author

It is firebase specific.

@ardatan
Copy link
Collaborator

ardatan commented Aug 5, 2019

@lookfirst So graphql-scalars is a package that includes more generic scalars. It would be better to have this scalar implementation in another npm package under your name in my opinion.

@ardatan ardatan closed this as completed Aug 5, 2019
@Urigo
Copy link
Owner

Urigo commented Aug 5, 2019

I mean, we can add a firebase-timestamp here if you think it would be valuable for more people

@lookfirst
Copy link
Contributor Author

Thanks @Urigo, don't worry about it. I was just seeing if you were interested. It is fine if not. I actually thought more about things and decided to store Date.toISOString() as a string in Firestore anyway. It just simplifies things.

@moritzjacobs
Copy link

Thank you, @lookfirst – you just saved me tons of time!

@simenandre
Copy link

simenandre commented May 31, 2020

Hello! I've made a separate package to handle this. It's based on graphql-scalars, so it would be easy to import into this package if it ever feels right. Meanwhile, people can use this. https://github.com/cobraz/firestore-graphql-scalars

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

No branches or pull requests

5 participants