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

Shared: Added typescript for times helper #17

Open
wants to merge 6 commits into
base: development
Choose a base branch
from

Conversation

promaty
Copy link
Contributor

@promaty promaty commented Nov 4, 2020

Starting with a single lib now, will add others in following PRs

@promaty promaty self-assigned this Nov 4, 2020
@@ -1,7 +1,7 @@
import expressSession from 'express-session'
import { Session } from '../models/objection'

import { HOURS, DAYS } from '@aragon/protocol-backend-shared/helpers/times'
import { HOURS, DAYS } from '@aragon/protocol-backend-shared/build/helpers/times'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may be a dumb question, but when we update these other helpers to be in TS, would we be able to import the source files?

It seems a bit weird to have to do this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One way to do it would be to define main and types in package.json like I did in ipfs pinner and then have all imports in a single index.ts file:
https://github.com/aragonone/ipfs-pinner/blob/development/packages/shared/src/index.ts

In this case we could import the same thing like this:

import { times } from '@aragon/protocol-backend-shared
const { HOURS, DAYS } = times

I find this more verbose however and you also have to think about multiple imports/exports not just the source file directly.

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.

2 participants