google / lovefield

Lovefield is a relational database for web apps. Written in JavaScript, works cross-browser. Provides SQL-like APIs that are fast, safe, and easy to use.
https://google.github.io/lovefield/
Apache License 2.0
6.81k stars 366 forks source link

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

Open luchillo17 opened 7 years ago

luchillo17 commented 7 years ago

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 commented 7 years ago

you can try:

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

....
luchillo17 commented 7 years ago

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?

https://github.com/DefinitelyTyped/DefinitelyTyped/issues/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 commented 7 years ago

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:

luchillo17 commented 7 years ago

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

ghost commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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

ghost commented 7 years ago

--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 commented 7 years ago

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

arthurhsu commented 7 years ago

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

ghost commented 7 years ago

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: https://github.com/google/lovefield/issues/188#issuecomment-269675366 Our version of the types is here.

arthurhsu commented 7 years ago

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.