pateketrueke / yrv

Your routing vibes! (for Svelte)
https://svelte.dev/repl/0f07c6134b16432591a9a3a0095a80de
161 stars 9 forks source link

Typescript support and warnings on build #65

Closed VenatusSimpleX closed 3 years ago

VenatusSimpleX commented 3 years ago

Hey! Thanks for the package, it was a delight to use it due to it's simplicity.

However, I've run into a few problems here, it's not exactly show-stopping, but maybe it's something that may require some attention from you.

First thing is typescript support, I wonder if there is typescript support for yrv? Running yarn validate will give hints/errors that there are no type definitions for yrv, and I tried doing yarn add --dev @types/yrv too, but it seems that it does not exists. image

Another thing are warnings on build, it seems like after a recent update, yrv is doing something that may have unintended consequences, I wonder if this is also something that may need your attention? This can be triggered with yarn build or yarn dev. image

Please do let me know if there is anything else you need from me, thanks!

pateketrueke commented 3 years ago

Oh, nice catches, I think later warnings are just fine, I'll find a way to tell the svelte-compiler to do not care.

For the typescript definitions I hope dts-gen could help us to generate those, you have any experience on that?

VenatusSimpleX commented 3 years ago

Unfortunately I don't have experiences in any of those, I've taken a look at dts-gen and it does seem like it generates a starting point for the definitions. Any more than that and it's out of my depth at the moment.

pateketrueke commented 3 years ago

Yeah, I feel the same way, I'm not sure if we can get typedefs working easily; i'll ask for help on this one, thank you!

VenatusSimpleX commented 3 years ago

Not sure if this is the right direction but I'm just digging in to see if we can nudge this in the right direction:

With dts-gen I would get errors like image

I tried another method, which was to install typescript in yrv, and then running the src/lib into tsc and see what it outputs.

tsconfig.json:

{
  "compilerOptions": {
    "strict": true,
    "allowJs": true,
    "target": "es6",
    "declaration": true,
    "declarationDir": "types",
    "emitDeclarationOnly": true
  },
  "include": ["./src/**/*"]
}

d.ts files generated: image

index.d.ts:

import { router } from "./lib/utils";
import { navigateTo } from "./lib/utils";
export { Router, Route, Link, router, navigateTo };

router.d.ts:

export function doFallback(failure: any, fallback: any): void;
export function handleRoutes(map: any, params: any): any[];
export function evtHandler(): void;
export function findRoutes(): void;
export function addRouter(root: any, fallback: any, callback: any): () => void;
export const baseRouter: any;
export const routeInfo: any;

utils.d.ts:

export function hashchangeEnable(value: any): boolean;
export function fixedLocation(path: any, callback: any, doFinally: any): void;
export function cleanPath(uri: any, fix: any): any;
export function navigateTo(path: any, options: any): void;
export function getProps(given: any, required: any): any;
export function isActive(uri: any, path: any, exact: any): any;
export function isPromise(object: any): any;
export function isSvelteComponent(object: any): any;
export const ROOT_URL: string;
export const router: any;
export const CTX_ROUTER: {};
export const CTX_ROUTE: {};
export let HASHCHANGE: boolean;

vendor.d.ts:

export {};

I think dts-gen is still the way to go, but that would assume that you have to change certain parts of your code to conform to typescript's standards, manually installing typescript instead (following this guide) does emit the type declaration files, so at least that's something.

jhechtf commented 3 years ago

I can see what I can do about this; I work with typescript quite frequently and I've helped with .d.ts files in other projects. Would we want to include the type definitions in the repo itself or through the DefinitelyTyped repo (e.g. @types/yrv)?

VenatusSimpleX commented 3 years ago

@pateketrueke Pinging you for attention

pateketrueke commented 3 years ago

Sorry for the late response, I would opt for both options, but if we can achieve that within the repo it'll be fine atm.

VenatusSimpleX commented 3 years ago

@jhechtf How are things? If you're too busy I don't mind trying to do this as I learn, though a direction would be nice.

jhechtf commented 3 years ago

I've been busy with work, but I have been trying to look up how to make typescript definitions for svelte. Haven't came up with a lot but I'll update again once I'm at home

On Tue, Feb 23, 2021, 19:04 VSX notifications@github.com wrote:

@jhechtf https://github.com/jhechtf How are things? If you're too busy I don't mind trying to do this as I learn, though a direction would be nice.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/pateketrueke/yrv/issues/65#issuecomment-784714873, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB6GLFZ7METGWXCZ3RI5E33TARUEHANCNFSM4XKIB7HQ .

jhechtf commented 3 years ago

Okay, I'll write this out really quickly JIC I get caught up in work stuff again.

I have a minorly working version, but haven't had a chance to actually dive into what it is. Not a whole lot of resources I've found for typing Svelte components that aren't written in TS so it's been weird.

the WIP I've got can be found on my fork of the repo at https://github.com/jhechtf/yrv/tree/wip/add-in-types. It's not all the way done, but I feel like it's most of the way done. I have to figure out how to properly type the let:router prop, as well as figuring out if we really need to type the slots (doesn't seem like it but didn't get a chance to look into it).

jhechtf commented 3 years ago

made a recent push to that branch for the types that adds in some JSDoc. I think they might be done but I'll have to double check in a project I have that actually uses YRV + TS to verify it fixes issues.