-
Notifications
You must be signed in to change notification settings - Fork 12
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
Octoherd integration #15
Comments
Most scripts should be able to run in the browser, they are built that way. So no need to run the script modules export the You can either send the requests directly from browser to GitHub's API or proxy them through your app. But I think the former is safer, because you'd never need to handle user access tokens in the backend Does that make sense? |
Sorry, I don't quite understand what you mean by this point perhaps due to my lack of web dev knowledge. Do you mean creating the OctokitJS instance at the frontend browser is safer than how I'm creating the OctokitJS instance at the API route? I thought usually making authenticated calls is safer at the backend than the frontend. |
Don't quote me on this, but I'm not sure if we do need a backend. If we have further plans to work with some user data, display logs or so, it might be necessary to call from the backend. |
Hello @Lofty-Brambles , I meant that the API routes are the backend here. In Next.js, the API routes are created and deployed as a combined Node.js serverless function. So in a way it is a 'backend', where users at the browser can't see any env values like |
I wouldn't say it's safer in every case. I'd say go whatever path works for you right now. Make it work, then make it great, and let the community help
that is a built-in web feature now, see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/import For example, the script at https://github.com/octoherd/script-hello-world is published to npm as https://www.npmjs.com/package/@octoherd/script-hello-world. You cannot import the module directly from npm yet, but you can use skypack, which wraps the npm registry and works great for that use case: https://www.skypack.dev/view/@octoherd/script-hello-world You can import that script statically like you'd traditionally do with const { script } = await import('https://cdn.skypack.dev/@octoherd/script-hello-world');
console.log(script) You can paste the above code in your developer tools console on a website like https://example.com/ which does not restrict imports from 3rd party domains (there are headers you can use to limit these, like github.com does it) Does that make sense? |
Okie got it! Thanks for the explanation @gr2m, now i have a better understanding of how to import and use the script 🙏 |
Hello @gr2m, while working on #42, I have tried to create an Octoherd-script using I did a workaround of by setting
|
Hm I'm not sure if that's possible. Skypack sends a For example https://cdn.skypack.dev/@octoherd/script-hello-world?dts sends
But https://cdn.skypack.dev/-/@octoherd/[email protected]/dist=es2019,mode=types/index.d.ts only states there is no type definition file. I think turning the JS Doc comments into types automagically is something that VS Code does, but only for locally imported modules. I'm not sure how things work when you import them from a URL. |
I see, then is it possible that in the octoherd-script that I've created, I change all the files there to |
I fear that won't work. I don't think that skypack compiles from TS to JS/DTS on-the-fly. You could probably add an But why exactly do you need the types? |
@gr2m I was thinking that the type safety will be good if I try to write code that calls the external skypark script 😁 but I guess for now its not that important yet. |
Description
Found out from @gr2m about the Octoherd project — i believe it can be a really cool feature to have at Octokit-lite to have an integration for Octoherd scripts.
Suggested solution for integrating Octoherd
Frontend
Backend
Docs
To work on this epic
All changes should be merged into the branch
feature/octoherd
, only when everything is done then the branch can be merged into themain
branch.The text was updated successfully, but these errors were encountered: