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

Better type declaration for inputAttributes #467

Merged
merged 3 commits into from Mar 14, 2017

Conversation

toverux
Copy link
Member

@toverux toverux commented Mar 13, 2017

This is a PR to improve the type definition of the inputAttributes option, added recenlty by @Fkscorpion in #466.

First, in the aca4dc3 commit, I used a mapped type - to allow IDEs to provide auto-completion - in intersection with a standard dictionary, to allow custom attributes and not only HTMLInputElement's ones.

Then, I've decided to remove the mapped type and only keep the dictionary, since this is a bleeding-edge, TypeScript 2.1 feature, so it would break alot of consumers' applications without a single benefit from a static typing point of view (only IDE assistance, but even my IDE is not yet capable of understanding a type intersection with a mapped type, so...).

You'll note that the dictionary maps to string and not any, because setAttribute() (used under the hood in SweetAlert) takes a string, and not any value (the DOM API, however, takes typed values, but setAttribute() is an HTML thing).

Maybe I will make a new PR about adding a inputProperties option that sets HTMLInputElement DOM properties, and not HTML attributes. The DOM API is better to work with than the XML/HTML one, fits most use cases and offers a much better typed experience.

API proposal:

interface SweetAlertOptions {
    inputProperties?: Partial<HTMLInputElement>;

    // de-sugarified:
    inputProperties?: { [P in keyof HTMLInputElement]?: HTMLInputElement[P] };
}

@limonte
Copy link
Member

limonte commented Mar 14, 2017

Thanks a million @toverux for caring about the TypeScript support. You're the best here! Feel free to merge PRs related to TypeScript without waiting for approval from me :)

Btw, I just noticed that definitions of 2 parameters are missing:

Could you please handle them?

@limonte limonte merged commit b881cbe into master Mar 14, 2017
@limonte limonte deleted the feature/typings-better-inputAttributes branch March 14, 2017 07:45
@toverux
Copy link
Member Author

toverux commented Mar 14, 2017

Thank you :)

Could you please handle them?

Of course, no problem!

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