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

Create Vercel app for onefetch with ASCII preview #701

Merged
merged 70 commits into from
Jul 29, 2022
Merged

Create Vercel app for onefetch with ASCII preview #701

merged 70 commits into from
Jul 29, 2022

Conversation

spenserblack
Copy link
Collaborator

@spenserblack spenserblack commented Jul 15, 2022

At the time of creation of this PR, there is absolutely nothing notable, hence why it is starting as a draft. I am creating this before any significant code is contributed so that I can document the steps as I go through them.

Setup

  1. Create a Vercel account at vercel.com and link it to GitHub
  2. Create a new project
    • set repo to onefetch
    • change the root directory from ./ to vercel/ascii-preview vercel/
    • set the framework preset to Vite
  3. Change the domain in project settings(?)

Resolves #696

@spenserblack
Copy link
Collaborator Author

I think I'll end up using TypeScript for this (using Vite as a build tool). Before I set up the preview, @o2sh do you have a preference between vanilla JS, React, Vue.js, Svelte, or anything else? I've used Vue.js and Svelte before (and obviously vanilla JS 😆), but I'm happy to use any tool that you and the team feel most comfortable with.

@spenserblack spenserblack changed the title Create folder for vercel app Create automated preview of ASCII art Jul 15, 2022
@spenserblack
Copy link
Collaborator Author

Also, if we ever want other serverless onefetch web services, maybe it makes more sense to make the root directory vercel/ instead, and /ascii-preview can be an endpoint instead.

@codecov-commenter
Copy link

codecov-commenter commented Jul 15, 2022

Codecov Report

Merging #701 (7b8a2ea) into main (debd453) will increase coverage by 0.17%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #701      +/-   ##
==========================================
+ Coverage   13.36%   13.53%   +0.17%     
==========================================
  Files          19       19              
  Lines        1160     1145      -15     
==========================================
  Hits          155      155              
+ Misses       1005      990      -15     
Impacted Files Coverage Δ
src/info/mod.rs 0.00% <0.00%> (ø)
src/info/repo.rs 16.66% <0.00%> (+1.32%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update debd453...7b8a2ea. Read the comment docs.

@@ -5,11 +5,13 @@ on:
paths-ignore:
- "docs/**"
- "**.md"
- "vercel/**"
Copy link
Owner

Choose a reason for hiding this comment

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

This folder may need to be added to the exclude section of the Cargo.toml file.

@o2sh
Copy link
Owner

o2sh commented Jul 15, 2022

Before I set up the preview, @o2sh do you have a preference between vanilla JS, React, Vue.js, Svelte, or anything else?

Why the need of a front end framework/library for a serverless function?

Also, if we ever want other serverless onefetch web services, maybe it makes more sense to make the root directory vercel/ instead, and /ascii-preview can be an endpoint instead.

Aren't Vercel's serverless functions supposed to be placed in the /api directory of the project's root. https://vercel.com/docs/concepts/functions/serverless-functions#deploying-serverless-functions

@spenserblack
Copy link
Collaborator Author

Why the need of a front end framework/library for a serverless function?

For ASCII previews itself, no big reason, just that it might be a bit easier to parse data into a template this way. Also, reactively changing the background from dark to light can be useful. While not impossible without a frontend framework, it's a bit easier, and also more readable for other potential contributors. Though if we decide to use any framework now, this simplifies adding anything else to a serverless web service. For example, #459 (comment) was suggesting moving the wiki to gh-pages, but something very similar can be accomplished using Vercel instead. Though I have no strong feelings, and would be happy to implement this with Vanilla JS (no frameworks/libraries).

Aren't Vercel's serverless functions supposed to be placed in the /api directory of the project's root.

Well, the serverless function don't need to be in /api. For example, github-profile-trophy is basically a single serverless function run with Deno that returns an SVG, and this serverless function exists in the root. E.g. https://github-profile-trophy.vercel.app/?username=o2sh (congrats on the achievements 😆 )

Personally, I'd put machine-readable responses in /api[:/endpoint], and human-readable responses anywhere else. We can split this up into, let's say, /api/languages which serves a JSON-ified version of languages.yaml, and /ascii-preview that fetches and consumes that JSON.

I think I may have been incorrectly using the phrase "serverless function." I was using it as a catch-all for static sites as well. Sorry for the confusion.

@o2sh
Copy link
Owner

o2sh commented Jul 16, 2022

While not impossible without a frontend framework, it's a bit easier, and also more readable for other potential contributors. Though if we decide to use any framework now, this simplifies adding anything else to a serverless web service.

You're right about the need of being future proof.
I'm always anxious about bringing unnecessary complexity to a project. 😩
Concerning your original question - which framework/library to use -, you can pick the one you feel most comfortable with and is most adapted to the feature. I have a small preference for (Svelte + TypeScript) 😉 .

Personally, I'd put machine-readable responses in /api[:/endpoint], and human-readable responses anywhere else. We can split this up into, let's say, /api/languages which serves a JSON-ified version of languages.yaml, and /ascii-preview that fetches and consumes that JSON.

If we're adding folders to the project's root, we should keep it to a minimum. For example, a single folder named tools - or another name that better illustrate our intent - which can then hold as many folders as we consider necessary.

@spenserblack
Copy link
Collaborator Author

If we're adding folders to the project's root, we should keep it to a minimum. For example, a single folder named tools - or another name that better illustrate our intent - which can then hold as many folders as we consider necessary.

Oh yeah, right now I'm planning on putting everything in vercel/, so vercel/api/, vercel/index.html, etc. I think that's what you're saying you want, right?

@spenserblack spenserblack marked this pull request as ready for review July 19, 2022 15:06
@spenserblack
Copy link
Collaborator Author

So I may have gone overboard 😆

Preview is at https://onefetch-spenserblack.vercel.app/

@o2sh
Copy link
Owner

o2sh commented Jul 27, 2022

TypeScript is really giving me a hard time with this import 😓

Screenshot 2022-07-27 at 23 04 20

seems like it doesn't want me to import a file that is one level up from its root path 🤔

@spenserblack
Copy link
Collaborator Author

TypeScript is really giving me a hard time with this import

I had that issue too, which is why I caved in and made the YAML -> JSON a build step. TypeScript natively supports importing JSON, but the YAML import is handled by a Vite plugin, so I'm not sure if the TypeScript compiler is aware of that.

I think something like this might be necessary.

// types.d.ts
declare module '*.yaml' {
  const data: Record<string, any>;
  export default data;
}

@spenserblack
Copy link
Collaborator Author

c93ef82 Resolved the type errors for me (running npm run check).

If a user runs `npm run build` locally to build the Vite deployment,
this folder will be generated with built assets. We probably don't want
those assets to be tracked.
@spenserblack
Copy link
Collaborator Author

🤔 I wonder if running npm run check should be made part of the CI. npm run build would be asserted by Vercel, so that might not be necessary.

@spenserblack
Copy link
Collaborator Author

😢 Looks like nightly had a change that broke the CI.

Copy link
Owner

@o2sh o2sh left a comment

Choose a reason for hiding this comment

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

I think the PR is ready to be merged 🎉

Maybe we can wait for gitoxide to fix the build issue 🤔 their CI is also failing

* ================
* Minimal css theme.
* Project: https://github.com/oxalorg/sakura/
*/
Copy link
Collaborator Author

@spenserblack spenserblack Jul 29, 2022

Choose a reason for hiding this comment

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

Since this has been modified by us, we might want to add a comment here just saying so. We don't need to, but it might be helpful for someone stumbling on this.
Sorry, ignore this, I didn't see 0179115 and thought that the colors were changed manually.

@spenserblack
Copy link
Collaborator Author

I think the PR is ready to be merged

I think so, too! I'm fine to wait until the CI is fixed. Thanks for all your work updating the styling, simplifying the logic, etc.!

@spenserblack
Copy link
Collaborator Author

🤔 Maybe we should add vercel/ to CODEOWNERS.

@o2sh o2sh merged commit a2d6cf1 into o2sh:main Jul 29, 2022
@spenserblack
Copy link
Collaborator Author

Should onefetch.dev be listed as one of the sites using sakura.css?

@o2sh
Copy link
Owner

o2sh commented Jul 29, 2022

Sure! that's a good idea 👍

@spenserblack
Copy link
Collaborator Author

oxalorg/sakura#97

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.

Automated logo rendering/visualization
3 participants