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

adding support for standalone npm package #12

Merged
merged 7 commits into from
Apr 11, 2023

Conversation

kauderk
Copy link
Contributor

@kauderk kauderk commented Feb 26, 2023

I've created a standalone npm package for the youtube-browser-api and made the necessary changes to the original repository to support it. This allows developers to use the API without having to set up a SvelteKit project.

Here are the commits included in this pull request:

Please let me know if you have any questions or concerns.

@Refzlund
Copy link
Owner

Thanks! Since my last commits few months ago I see that svelte-package updated to 2.0.1 rendering my helper functions useless. So I'mma just merge this and clean the project up a bit.

Thank you for helping me out!

@Refzlund Refzlund merged commit 54f91d2 into Refzlund:master Apr 11, 2023
@Refzlund
Copy link
Owner

@kauderk Would you mind seeing if the standalone for your project works with the latest version?

@kauderk
Copy link
Contributor Author

kauderk commented Apr 13, 2023

  • I couldn't compile to the latest version, because my packages depend on typescript below 5.0

    npm log
    youtube-browser-api> pnpm i sveltekit-zero-api@latest -D
    Already up to date
    Progress: resolved 361, reused 339, downloaded 0, added 0, done
    WARN  Issues with peer dependencies found
    .
    └─┬ sveltekit-zero-api 0.13.7
    └── ✕ unmet peer typescript@^5.0.4: found 4.9.5
    youtube-browser-api> pnpm install typescript@^5.0.4 -D
    Packages: +11 -9
    +++++++++++---------
    Progress: resolved 362, reused 340, downloaded 0, added 0, done
    
    devDependencies:  
    - typescript 4.9.5
    + typescript 5.0.4
    
    WARN  Issues with peer dependencies found
    .
    ├─┬ @sveltejs/package 2.0.2
    │ └─┬ svelte2tsx 0.6.1
    │   └── ✕ unmet peer typescript@^4.9.4: found 5.0.4
    └─┬ svelte-preprocess 5.0.1
    └── ✕ unmet peer typescript@"^3.9.5 || ^4.0.0": found 5.0.4
    youtube-browser-api> 
  • I created a patch that helped me to publish my package. Seems that I didn't include it in the PR

    patch

    One makes sure that my custom url gets used

    -    const url = options.config.baseUrl || '' + options.path + ('query' in api ? '?' + new URLSearchParams(api.query).toString() : '');
    +    const url = (options.config.baseUrl || '') + options.path + ('query' in api ? '?' + new URLSearchParams(api.query).toString() : '');

    And the other that my fetch/GET gets handled by the client's browser.

    -    const response = fetch(url, requestInit);
    +	// avoid making the "preflight http request", which will make it twice as fast
    +	const collapsedRequestInit = api.method === 'GET' ? undefined : requestInit
    +
    +    const response = fetch(url, collapsedRequestInit);
  • I saw that you used the new "const" typescript keyword, are you planning for generics or branches on the Ok method?

@Refzlund
Copy link
Owner

  1. I believe you can upgrade packages mentioned. A worthy side-note is that TypeScript does not use semantic versioning (5.0.0 is basically like 4.10.0), so you should be able to upgrade without anything breaking as far as I'm aware, even if they're stating a peer dependency of ^4.x.x. Let me know if that's not the case. I can revert back to typescript 4.8/4.9 if needed.
  2. If you're referring to patches/sveltekit-zero-api@0.12.1.patch It seems to me it was included in the latest version of sveltekit-zero-api
  3. I honestly just dropped typescript const in there to make sure the types were as expected when I was fixing an issue with types not always going through. In the context of Ok, am I missing something? If you can give me an example of the behaviour you expected/want/need, then I can probably look into implementing it

@kauderk
Copy link
Contributor Author

kauderk commented Apr 16, 2023

I want to process generics on my endpoints, I've been working on a schema that maps generic types: here https://stackblitz.com/edit/github-bifnc9?file=package.json,src%2Froutes%2Fquery%2Fdemo.ts
I think the GET/POST methods should take generics https://stackblitz.com/edit/github-bifnc9?file=package.json,src%2Findex.ts
Btw I did manage to upgrade my project to ts-5

@Refzlund
Copy link
Owner

Ah okay, so you something along the lines of this?

// api/query/+server.ts
export async function GET<const Q>(event: API<{ query: Q }>) {
    return Ok({
        body: {...} as Q
    })
}

// somewhere.js
api.query.GET({  query: { something: 'abc' as const }  })
    .Ok(res => {
        res.body
             // ^? body: Q = { something: 'abc' }
    })

@kauderk
Copy link
Contributor Author

kauderk commented Apr 16, 2023

  • Yes something like this
    • Speaking about that generic Flatten utility, do you know of anything similar? Has someone already figured this out?
  • I did try to do this: screenshot
    But it got too confusing and hard to debug because I had to build on each change to see the effects. Is this the way you do it?

@Refzlund
Copy link
Owner

  • Yes something like this

    • Speaking about that generic Flatten utility, do you know of anything similar? Has someone already figured this out?
  • I did try to do this: screenshot
    But it got too confusing and hard to debug because I had to build on each change to see the effects. Is this the way you do it?

I see! So for the most recent version of sveltekit-zero-api you can use the package script;

  1. Open terminal: pnpm package
  2. Open a new terminal: pnpm dev

package-script will watch any file changes and re-build whenever, so debugging should be much smoother. dev runs the SvelteKit project updating API's when you create/edit them.

In b788348 where I fixed an issue, you can see I created a minimal reproduction inside the package so I could see how changes would reflect immediately. Let me know if I can help🦒

@Refzlund
Copy link
Owner

@kauderk Do you mind sharing what you've made? I considered the suggestion and it's probably not straight forward.

The input and the returned type from the Rest API method are separate like so:
image

So the question becomes how to make them correlated in a meaningful way.

I sort of have an idea for an approach, and I could probably look into it.

@Refzlund
Copy link
Owner

You can't really bridge the gap directly, as the generic on the GET method:
image

You'd have to do something like:
image

Which I feel like is somehow possible

@kauderk
Copy link
Contributor Author

kauderk commented Apr 16, 2023

I wanted to track down the types so I decided to use import/export types instead of references https://github.com/kauderk/sveltekit-zero-api/tree/generics/src/lib/types
Although I tried, I didn't make much further progress.

@Refzlund
Copy link
Owner

I wanted to track down the types so I decided to use import/export types instead of references https://github.com/kauderk/sveltekit-zero-api/tree/generics/src/lib/types Although I tried, I didn't make much further progress.

Yeah understandable. And the refactoring makes sense. I was learning about ambient types and referencing types when I was writing that part — and have since learned that was not the way to do it.

I'll see what I can do about the generic. I'm trying to do

interface Post {
    query: { ... }
}
export function POST<const R extends Post>(event: API<R>) {
    ...
}

Where R will be the RequestInit input (GET({ ... }))

@Refzlund
Copy link
Owner

I'm waiting for some help on the TypeScript community on Discord

I made a TypeScript Playground if you have any ideas to try out

@Refzlund
Copy link
Owner

The best attempt we could do as stated in the Discord conversation is to handle the typed API like

export type GeneratedAPI = {
    users: {
        GET: Z<typeof userEndpoint.GET<{...}>>
    }
}

Honestly that's the closest solution I can think of. And yet, I don't know how we would pass the RequestInit inside here.
Probably something along the lines of
GET: <T extends ...>(requestInit: T, fetch) => Z<typeof userEndpoint.GET<T>>

There's no easy solution atm anyways🦒

@kauderk
Copy link
Contributor Author

kauderk commented Apr 17, 2023

This seems like it will it requiere some heavy "type drilling".
The typeof FakeFunc seems to be of help.
This all has me thinking about private types.
Why not rewrite the types into a ApiTypeFunction so that the Generics are preserved all the way through.
I did something similar on my Flatten type (end of file)
"Generics cannot exist at the scope of objects, they have to exist within a function." https://youtu.be/xk_PbxR7G8A?t=3859
This is a challenge

@Refzlund
Copy link
Owner

This seems like it will it requiere some heavy "type drilling". The typeof FakeFunc seems to be of help. This all has me thinking about private types. Why not rewrite the types into a ApiTypeFunction so that the Generics are preserved all the way through. I did something similar on my Flatten type (end of file) "Generics cannot exist at the scope of objects, they have to exist within a function." https://youtu.be/xk_PbxR7G8A?t=3859 This is a challenge

Yeah exactly. I think we have to "solidify" the types from where they are imported (inside sveltekit-zero-api.d.ts). I made a branch to show you what I was thinking we could do: https://github.com/Refzlund/sveltekit-zero-api/blob/Generic-Endpoints/src/routes/test.ts

So...

  1. Use RegEx to find generic endpoint methods in a file
  2. Handle generic endpoint methods like shown in the test.ts file
  3. Omit those generic Rest Methods from the Z<...> which will handle the non-generic endpoint methods

The only concerning thing I have is the Regex as I don't know how that will impact the performance. I wouldn't personally care too much really, so I think it would be a valid strategy.

@Refzlund
Copy link
Owner

image

This is going to be the implementation hope you're fine with that🕺 I'll see if I can finish it today

@Refzlund
Copy link
Owner

Done #15 (comment)

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.

2 participants