search

wikimedia-gadgets / types-mediawiki

TypeScript definitions for MediaWiki JS interface
GNU General Public License v3.0
23 stars 8 forks source link

Type mw.cookie #7

Closed cAttte closed 3 years ago

cAttte commented 3 years ago

added typings for mw.cookie! i don't know how you feel about set()'s options.sameSite? i made it case-insensitive to stick to the actual values accepted at runtime, and since some people may prefer lowercase, startcase, or uppercase strings for this type of stuff. but it could be overkill? though it's not a lot of code anyways, just one useless type parameter

siddharthvp commented 3 years ago

mw/index.d.ts will need an import for the new file.

i don't know how you feel about set()'s options.sameSite? i made it case-insensitive to stick to the actual values accepted at runtime, and since some people may prefer lowercase, startcase, or uppercase strings for this type of stuff. but it could be overkill?

Seems fine to me - is there any disadvantage?

cAttte commented 3 years ago

sorry, i added the import. but apparently, those aren't even needed? (which is why i forgot). like, try removing all imports from the index and using namespaces declared by other files, and it should still work. though i'm not sure if that's just the TypeScript compiler or VS Code, so it's fine to leave it just in case.

is there any disadvantage?

nope, i just thought it was kind of weird to have to use a type parameter for an insignificant option like that, but there aren't any disadvantages.

siddharthvp commented 3 years ago

sorry, i added the import. but apparently, those aren't even needed? (which is why i forgot). like, try removing all imports from the index and using namespaces declared by other files, and it should still work. though i'm not sure if that's just the TypeScript compiler or VS Code, so it's fine to leave it just in case.

Hmm. I see that if the consumer project tsconfig.json has "include": "./node_modules/types-mediawiki" then they'ren't needed, but if it has "include": "./node_modules/types-mediawiki/index.d.ts" then they're needed. I have been doing the latter in my projects, though I mentioned the former in the README!

cAttte commented 3 years ago

ohh, alright, that makes sense