Skip to content
This repository has been archived by the owner on Jan 10, 2023. It is now read-only.

[Bug][Types] 'lovefield.d.ts' is not a module #188

Open
luchillo17 opened this issue Dec 27, 2016 · 12 comments
Open

[Bug][Types] 'lovefield.d.ts' is not a module #188

luchillo17 opened this issue Dec 27, 2016 · 12 comments

Comments

@luchillo17
Copy link

This is what VSCode shows when importing lovefield, in my repo it shows module '*' if can't find, if i get into the dist folder then shows the error in the image.

image

@Brooooooklyn
Copy link

you can try:

npm i @types/lovefield --save-dev
import * as lf from 'lovefield'

....

@luchillo17
Copy link
Author

luchillo17 commented Dec 28, 2016

I did try the first one, when i was providing it as a global variable with Webpack's ProvidePlugin and it didn't work.

As for the second it worked very well, i realized later that if it was an UMD module i had to import like that, then the typings started working, however i still have the doubt, why does the one inside the lovefield package differs from the one in DefinitelyTyped? lack of update?

DefinitelyTyped/DefinitelyTyped#13574

Also take a look to this line in the typings of the repo, it seems like you want to get the typings of es6-promise, however in my case i use core-js to provide such polyfill for promises, shouldn't it be optional which promise polyfill to use? , also it seems it's being grabbed from the wrong folder, current one goes to the parent of dist, that is the lovefield folder, not the node_ modules one when installed, so i think it will not find the es6-promise file referenced: https://github.com/google/lovefield/blob/master/dist/lovefield.d.ts#L20

@ghost
Copy link

ghost commented Dec 29, 2016

To bundle types, you should add "types": "./dist/lovefield.d.ts" to your package.json.
When types are bundled, we can remove lovefield from DefinitelyTyped and deprecate @types/lovefield in favor of just lovefield.

A few notes about improving the definition:

  • Instead of using /// <reference path="../es6-promise/es6-promise.d.ts"/>, just require that people use --lib es6.
  • Instead of declare module 'lf', add this at the top level to allow importing:
export = lf;
export as namespace lf;

@luchillo17
Copy link
Author

@andy-ms What do you mean to use --lib es6?

@ghost
Copy link

ghost commented Dec 29, 2016

The --lib compiler option controls what types are available by default. --lib es6 includes (among others) types for Promise. (In tsconfig.json this would be "lib": ["es6"].)

@luchillo17
Copy link
Author

I meant to ask, is that an option in the library repo or is it in the client project that uses this lib?

For example i use webpack, and i don't recall this option being available.

@luchillo17
Copy link
Author

Aside in my project i provide promises polyfill through core-js so i don't want to install es6-promise if not needed.

@ghost
Copy link

ghost commented Dec 29, 2016

--lib is a TypeScript option, not a webpack option. Whatever build tool you use should give you the ability to specify TypeScript options.
When a library depends on ES6 things like Promise, generally it is the library user's responsibility to make a Promise shim (if necessary) and types available -- not the library's responsibility.
And of course, removing the reference to es6-promise will mean that you don't need to have it installed.

@luchillo17
Copy link
Author

Yeah, i'm already making use of a polyfill that takes care of Promise's shimming, and most likely other users will.

@arthurhsu
Copy link
Contributor

Closing this issue since there's no actionable item from Lovefield library.

@ghost
Copy link

ghost commented Jan 4, 2017

Please re-open. The typings are currently unusable and they are easy to fix. Don't know why you think there is no actionable item, since I did mention a few. The most important one is to add "types": "./dist/lovefield.d.ts" to your package.json.
Specific comment link: #188 (comment)
Our version of the types is here.

@arthurhsu
Copy link
Contributor

Ok, I thought you were talking about adding to Luchillo's stuff (since the "your" looks like replying his post). TBH Lovefield team will not be able to test that npm package.json and we don't know if that would create side effects.

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

No branches or pull requests

3 participants