Typings for TypeScript about css-vars-ponyfill HOT 17 CLOSED

Node types are more accurate, you could make them like that. As for testing, I frankly wouldn't bother as types here are rather clear and simple. You're not declaring you own types or anything like that, you just use primitives and interfaces that cannot be anything other than what you wrote.

from css-vars-ponyfill.

@waterplea --

Fixed in 1.16.0.

Thanks for the heads up, btw. I told you I was a TS rookie. 😉

from css-vars-ponyfill.

Hi @waterplea,

I'm have never used TypeScript, so I would need some time to familiarize myself with it before I feel comfortable adding TypeScript-related code to the project. Unfortunately, extra time isn't something I don't have a lot of these days, so I can't guarantee if/when I will be able to get around to it.

Two quick questions:

1. Is this required for the ponyfill to function in a TypeScript environment?
2. Is this an urgent need, or a "nice to have" that can wait for a future release?

Thanks for taking the time to share the TypeScript definition and create the issue. Very much appreciated!

from css-vars-ponyfill.

Well, TypeScript basically just has types for variables so in order for it to know how to work with your library, you would need to tell it what are they. That's what I did with my snipped above. If your library only exports this cssVars function it should really be enough to just add css-vars-ponyfill.d.ts file to the library and properly add it to package.json.

As for your questions:

1. This is required to use ponyfill in TypeScript, otherwise it would give you an error. Which leads to the second question:
2. An error could probably just be suppressed somehow and the ponyfill would work since it's just a static type check, but the proper way to do it would be to add the snipped I have posted to every project using ponyfill, which is what I did for the two projects I'm working on for the time being.

So yes, that's "nice to have". Problem is, I'm working on a library which means that every TypeScript project that would use my library would have to also add that file, unless I figure out how to include it with the library or you add it 😉

from css-vars-ponyfill.

Hi again, @waterplea.

Haven't forgotten about you. I've left this issue open as a reminder to investigate TypeScript to the point where I feel comfortable checking in the .ts file. Not sure when that will be, but hopefully sooner than later.

I hope you've managed to move forward while waiting for me. If not having the .ts file in place is a blocking issue for you, let me know and I'll see what I can do.

from css-vars-ponyfill.

Typings are absolutely not necessary to use it in Typescript unless you set your compile to not allow these things (just to clear this up).

Also, there's the DefinitelyTyped projects providing typings for libraries externally (but admittedly having it baked into the library is preferable).

from css-vars-ponyfill.

Two quick updates...

The 1.8.0 release adds a new watch option (boolean), so TypeScript user will want to update their definitions when upgrading to this version.

Also, I've decided against adding the typescript definitions to the project. Apologies to @waterplea, but the addition of the watch function made me realize that changes I make to the ponyfill may require updates the TypeScript definitions as well. With the new watch feature, it looks like this would be a simple matter of adding watch?: boolean;. Easy enough, but I have no way of verifying that the change works, and major lib changes may require more substantial updates to the .d.ts file that I am not in a position to make.

TypeScript is on my list of things to check out, but as I mentioned earlier I'm not sure when (or if) this will happen. If I do find the time, I will happily add a TypeScript file to the ponyfill once I feel comfortable maintaining it. Until then, perhaps @Airblader's DefinitelyTyped suggestion is the best way to go.

Thanks for the thoughtful discussion, fellas. Very much appreciated.

from css-vars-ponyfill.

That's cool, we've configured our library to include the typings so it's no bother for our users. Yes, you would need them updated every time open API change if you ever decide to add them.

BTW, I've made my own MutationObserver approach to use your ponyfill with Angular, where styles always change. Is you "watch" option optimized? Does it ignore STYLE tags with no use or declarations of variables? Does it go over everything everytime new style is added, or does it somehow determines what has to be updated and what could be left untouched? What I did was I've added data- attributes to style tags where variables were declared so that I would always include those with ponyfill, then I would monitor new STYLEs content for indexOf('--') to know if there are variables involved. And I would not go over old styles if there were no declaration of variables in new STYLEs. Don't know if that's any helpful to you, just wanted to share my thoughts on the matter and thanks for considering my issue.

Keep up the great work!

from css-vars-ponyfill.

Good to hear you've baked the typings into you library without too much trouble.

As for the MutationObserver implementation, it has been optimized. You can review the code here:

Here's a quick summary of the optimizations:

1. Verifies that the mutation target is an appropriate <link> or <style> node.
2. Ignores <style> mutations that do not contain a CSS custom property declaration (e.g. --mycolor: red) or function (e.g. color: var(--mycolor);).
3. Ignores all attribute mutations except disabled and href
4. Handles stylesheet enabled/disabled state
5. Debounces ponyfill update to prevent a high volume of mutations from creating a high volume of ponyfill calls.

There are more optimizations that could be done, but these made for a good first pass.

from css-vars-ponyfill.

Typings would be really easy to add into this repository, you just need to add the following line into your package.json: "types": "dist/css-vars-ponyfill.d.ts".

For css-vars-ponyfill.d.ts you can take the suggestion of @waterplea if it still up-to-date.

With that line added, TypeScript users should be able to see the typings ;)
I can issue a PR for that if you want.

from css-vars-ponyfill.

Thanks @dubzzz (and @waterplea for the initial .d.ts file).

I know close to zero about TypeScript, but based on what I've read would the following typings not be more accurate (specifically, the node type annotations on callbacks)?

declare module 'css-vars-ponyfill' {
    export default function cssVars(options?: {
        include?: string;
        exclude?: string;
        fixNestedCalc?: boolean;
        onlyLegacy?: boolean;
        onlyVars?: boolean;
        preserve?: boolean;
        silent?: boolean;
        updateDOM?: boolean;
        updateURLs?: boolean;
        variables?: {[key: string]: string};
        watch?: boolean;
        onBeforeSend?(xhr: XMLHttpRequest, node: HTMLLinkElement|HTMLStyleElement, url: string): void;
        onSuccess?(cssText: string, node: HTMLLinkElement|HTMLStyleElement, url: string): void;
        onError?(message: string, node: HTMLLinkElement|HTMLStyleElement, xhr: XMLHttpRequest, url: string): void;
        onWarning?(message: string): void;
        onComplete?(cssText: string, styleNode: HTMLStyleElement): void;
    }): void;
}

As for adding these to the project, I'm open to doing so if there is a relatively painless way I can test the type definitions before release. I understand that adding a file and an entry in package.json is easy, but my concern is that releasing incorrect type definitions could prevent a project that depends on this ponyfill from compiling properly.

Perhaps "testing your type definitions file" isn't something folks typically do, or less-then-perfect type definitions for third-party scripts happen all the time and devs know how to work around them. I dunno. I'm just not up to speed on TypeScript. What I can't do is copy-paste code into the repo that I'm unable to test or maintain. If the answer is "rewrite all of your tests in TypeScript", that's a larger block of work than just including a .d.ts file.

Happy to discuss further, and I'm open to any suggestions. I'd love to provide type definitions for TypeScript users, but that has to be a prioritized below allowing me to properly maintain the project.

from css-vars-ponyfill.

@jhildenbiddle @waterplea Seeing that the css-vars-ponyfill repo is getting quite a lot of NPM downloads as of late, do you think it makes sense if we try to published it to DefinitelyTyped? :)

from css-vars-ponyfill.

I'm less opposed to adding a .d.ts file to the repo than I was previously.

The definitions are pretty straightforward, and recent changes like the addition of options.rootElement, options.shadowDOM, and a third argument to the options.onComplete callback are reminders that they best way to keep the definitions up to date is if I maintain them.

If @waterplea, @terrymun, @Airblader or @dubzzz would test these definitions and confirm they work as expected, I will add them to the repo.

declare module 'css-vars-ponyfill' {
    export default function cssVars(options?: {
        rootElement?: HTMLElement|Node;
        include?: string;
        exclude?: string;
        fixNestedCalc?: boolean;
        onlyLegacy?: boolean;
        onlyVars?: boolean;
        preserve?: boolean;
        shadowDOM?: boolean;
        silent?: boolean;
        updateDOM?: boolean;
        updateURLs?: boolean;
        variables?: {[key: string]: string};
        watch?: boolean;
        onBeforeSend?(xhr: XMLHttpRequest, node: HTMLLinkElement|HTMLStyleElement, url: string): void;
        onSuccess?(cssText: string, node: HTMLLinkElement|HTMLStyleElement, url: string): void;
        onError?(message: string, node: HTMLLinkElement|HTMLStyleElement, xhr: XMLHttpRequest, url: string): void;
        onWarning?(message: string): void;
        onComplete?(cssText: string, styleNode: HTMLStyleElement, cssVariables: {[key: string]: string}): void;
    }): void;
}

One additional question: What happens if devs use the ponyfill properly but the type definitions are incorrect (e.g. a the definitions specify a string instead of a boolean, or a callback definition is missing an argument)? I'm assuming the answer depends on how devs have TypeScript configured (display an error vs throw an error). Since I'm not using these definitions, my concern has always been that I would update the ponyfill, forget to make necessary changes to the .d.ts file, and end up breaking a build for a TypeScript-enabled project.

from css-vars-ponyfill.

@jhildenbiddle You have definitely raised some valid points: if any breaking changes are made to a plugin's interface, then the typings will have to be updated and it might break compatibility with older versions.

I have actually used your definitions locally when importing css-vars-ponyfill in TypeScript, and they worked just as fine :)

In terms of deployment/hosting, it really depends on you: if you want it to be part of DefinitelyTyped or as a *.d.ts file in this repo 🥇

from css-vars-ponyfill.

Regarding your question — TypeScript are just static checks, in the end it will be just JS so it really depends on your code. If definitions specify string instead of boolean, then developers will see that they have to pass string and it will most likely be typecasted by JS in your code so any string, other than empty will pass as true. Frankly, .d.ts file with types it just a way for TypeScript developers to work with your library without constantly checking your git hub page for API :) It is practically the same so if you manage to keep them correct on the page — you will be able to handle the .d.ts file.

from css-vars-ponyfill.

Now available in 1.12.0.

Thanks for the discussion and help, fellas.

from css-vars-ponyfill.

Hey @jhildenbiddle, typings are there but are not automatically picked up. You need to add "types": "dist/css-vars-ponyfill.d.ts" to your package.json, I've added it manually next to "main" and it started working with all options and types suggestions which is sweet 👍

Please add it when you plan to update the library next time :)

from css-vars-ponyfill.