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

Export interfaces #983

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Export interfaces #983

wants to merge 1 commit into from

Conversation

wclr
Copy link

@wclr wclr commented Aug 9, 2020

Following my comment made four years ago =) #600 (comment)

Export interfaces for convenient driver creation.

Export interfaces for convenient driver creation.
thgreasi
thgreasi previously approved these changes Aug 10, 2020
@thgreasi thgreasi dismissed their stale review August 10, 2020 11:02

Tests don't pass

@thgreasi
Copy link
Member

The tests seems to not pass.
I think that b/c we are using a module definition (b/c of the UMD export we do), you can't use export at the top level and you need to define the interfaces inside the module definition.

@wclr
Copy link
Author

wclr commented Aug 10, 2020

Actually I just noticed that interfaces are available without the need to explicitly import. Which is not nice, but at least it works.

@wclr
Copy link
Author

wclr commented Aug 16, 2020

@thgreasi
though there is an issue with this

@wclr wclr closed this Aug 16, 2020
@wclr
Copy link
Author

wclr commented Aug 16, 2020

Closed by mistake. There is a real issue with current configuration. Because it allows to use modules even if not installed in (available for) particular package for example in the workspace project (when it is installed somewhere, and available everywhere). So anyway this should be changed.

@wclr wclr reopened this Aug 16, 2020
@thgreasi
Copy link
Member

Because it allows to use modules even if not installed in (available for) particular package for example in the workspace project

@whitecolor can you clarify what you mean with this a bit more? The installed part is what confused me.
Do you mean that in a project that you have LocalForage installed, you can use the interfaces even w/o importing or /// reference-ing them?

@wclr
Copy link
Author

wclr commented Aug 21, 2020

Do you mean that in a project that you have LocalForage installed, you can use the interfaces even w/o importing or /// reference-ing them?

I think everything is ok.

So one still have to import from 'localforage', they are not globally visible.

import type localforage from 'localforage'

const options: LocalForageOptions 

@thgreasi
Copy link
Member

Can you also try fixing the tests?
Otherwise I'm afraid we can't merge this.

@thgreasi
Copy link
Member

Let me know if you would prefer me or someone else to pick this up.

@wclr
Copy link
Author

wclr commented Aug 21, 2020

Let me know if you would prefer me or someone else to pick this up.

Yes, I don't know to fix the tests. As I said actually I'm ok with the current situation, exporting interface seems not actually necessary. But probably it would more correct to make them only available via explicit importing.

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.

None yet

2 participants