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

WIP: Typescript support #165

Merged
merged 23 commits into from Jul 25, 2020
Merged

WIP: Typescript support #165

merged 23 commits into from Jul 25, 2020

Conversation

BlackFenix2
Copy link
Contributor

i made a PR to enable typescript support writing the .svelte files and copied the type definitions straight from ReactStrap.

This isn't a complete repo, i just needed an area to converse about adding type definitions.

@BlackFenix2
Copy link
Contributor Author

@bestguy i forgot i have to remove all the references to react in the d.ts files. Stupid question, how can i publish this NPM package locally and make sure the type definitions work?

@bestguy
Copy link
Owner

bestguy commented Jul 19, 2020

You can use 'npm install /path/to/your/lib' for local testing

@BlackFenix2
Copy link
Contributor Author

ah, i figured it our earlier yesterday. i managed to get the intellisense working if i import the .svelte file directly from src for Sapper SSR, but i cannot get the proper type definition of a svelte component for the ES/UMD bundles

i continued the typescript definitions issue here: #129 (comment).

@BlackFenix2
Copy link
Contributor Author

@bestguy Maybe we should omit the type definitions from this PR, the intellisense will work if the consumer imports the '.svelte.' file directly for SSR to enjoy the intellisense. i just cant get type definitions for the es and umd bundles yet, that might be a whole new PR i need to research further.

i still have work to do, the intellisense works perfectly if your consuming project is also typescript (includes the typescript preprocessor), the build errors out when plain javascript is used. Maybe a config option in our rollup config can output compiled .svelte files.

button component

<script lang="ts">
  import classnames from './utils';
  type ButtonColor =
    | 'primary'
    | 'secondary'
    | 'success'
    | 'danger'
    | 'warning'
    | 'info'
    | 'light'
    | 'dark';

  type ButtonSize = 'lg' | 'sm';

  let className = '';
  export { className as class };
  export let active: boolean = false;
  export let block: boolean = false;
  export let children = undefined;
  export let close: boolean = false;
  export let color: ButtonColor = 'secondary';
  export let disabled: boolean = false;
  export let href: string = '';
  export let id: string = '';
  export let outline: boolean = false;
  export let size: ButtonSize;
  export let style = '';
  export let value = '';

  $: ariaLabel = $$props['aria-label'];

  $: classes = classnames(
    className,
    { close },
    close || 'btn',
    close || `btn${outline ? '-outline' : ''}-${color}`,
    size ? `btn-${size}` : false,
    block ? 'btn-block' : false,
    { active }
  );

  $: defaultAriaLabel = close ? 'Close' : null;
</script>

{#if href}
  <a
    {...$$restProps}
    {id}
    class={classes}
    {disabled}
    on:click
    {href}
    aria-label={ariaLabel || defaultAriaLabel}
    {style}>
    {#if children}
      {children}
    {:else}
      <slot />
    {/if}
  </a>
{:else}
  <button
    {...$$restProps}
    {id}
    class={classes}
    {disabled}
    on:click
    {value}
    aria-label={ariaLabel || defaultAriaLabel}
    {style}>
    <slot>
      {#if close}
        <span aria-hidden="true">×</span>
      {:else if children}
        {children}
      {:else}
        <slot />
      {/if}
    </slot>
  </button>
{/if}

button story

<script>
  // can import without requiring typescript language
// breaks if using rollup without a typescript preprocessor
  import { Button } from 'sveltestrap';
  const colors = [
    'primary',
    'secondary',
    'success',
    'danger',
    'warning',
    'info',
    'light',
    'dark'
  ];
</script>

{#each colors as color}
  <div>
    <Button {color}>{color}</Button>
  </div>
{/each}

The storybook config, build process and jest tests are all green. let me know if you need anymore details.

@BlackFenix2
Copy link
Contributor Author

i finally managed to make a Svelte component type definition! #129 (comment)

I restored the d.ts files and am changing each d.ts file by hand. this could take a while....

@BlackFenix2
Copy link
Contributor Author

@bestguy the typescript definition files are all done. you should be able to merge this PR without issues. if this works, well have the first svelte UI package with typescript support. 😄

let me know if you need anything else.

@bestguy
Copy link
Owner

bestguy commented Jul 22, 2020

Hey this is great @BlackFenix2, thanks!
I just saw the Svelte+Typescript post from Friday: https://svelte.dev/blog/svelte-and-typescript, would that change anything here?

@BlackFenix2
Copy link
Contributor Author

I can clone a sample of the project listed in the blog and see if we can enhance our typescript-ness. let me have a quick look and ill add another commit, then we can review the type definitions before merging.

- installed svelte-check for type checking
- removed svelte.config.js
- extended svelte tsconfig.json
- hotfix, changed input file in rollup to index.js
@BlackFenix2
Copy link
Contributor Author

@bestguy i had to move the type definitions outside of the src folder, im going through the svelte-check CLI to finish typechecking the src and stories components.

ill run a test build, story, jest and test the package locally. We should be good to merge after then, ill have the PR complete later today.

@BlackFenix2
Copy link
Contributor Author

@bestguy good news, the PR is ready for review! i tested the build,jest,storybook and tested it out on my sapper project.

other good news, i can import the 'sveltstrap' files without referencing the /src/* directory. a recent update to svelte-preprocess allows sapper projects to import the es modules without error! with this fix we no longer need to publish the .svelte files themselves, but i kept them intact for another PR.

let me know what you think!

@bestguy
Copy link
Owner

bestguy commented Jul 25, 2020

Thank you @BlackFenix2 ! I'm looking tor review and merge this weekend, nice work here.

Copy link
Owner

@bestguy bestguy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent, thanks for all the work on this

@bestguy bestguy merged commit 358466c into bestguy:master Jul 25, 2020
@BlackFenix2
Copy link
Contributor Author

wait a sec, i just downloaded the package for my project. i forgot to add '@types' in package.json.

 "files": [
    "dist",
    "src",
    "@types"
  ],

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