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.

Sign up for GitHub

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

Implement zGoogleFont zod type #4284

Draft
wants to merge 11 commits into
base: main
Choose a base branch
from
+76 −40
Draft

Implement zGoogleFont zod type #4284

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
39405fb
link @remotion/google-fonts
erayerdin Sep 5, 2024
26238d0
add zGoogleFont
erayerdin Sep 5, 2024
595b2e4
update lock file
erayerdin Sep 5, 2024
8abe7b2
add GoogleFontPicker to examples
erayerdin Sep 5, 2024
0ac605d
declare module for *.module.scss to fix pnpm build error
erayerdin Sep 5, 2024
e42ec6a
remove unused lint to fix pnpm build error
erayerdin Sep 5, 2024
687a5a2
remove unused input var to fix pnpm build error
erayerdin Sep 5, 2024
516b15f
add default value to zGoogleFont
erayerdin Sep 5, 2024
983d6c6
add comment to explain zod enums
erayerdin Sep 5, 2024
aaa6b97
use font prop in example composition
erayerdin Sep 6, 2024
611921a
Merge branch 'main' into pr/4284
JonnyBurger Sep 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion packages/zod-types/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@
"url": "https://github.com/remotion-dev/remotion/issues"
},
"dependencies": {
"remotion": "workspace:*"
"remotion": "workspace:*",
"@remotion/google-fonts": "workspace:*"
},
"peerDependencies": {
"zod": "3.22.3"
Expand Down
1 change: 1 addition & 0 deletions packages/zod-types/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import {parseColor, REMOTION_COLOR_BRAND} from './z-color.js';
import {REMOTION_TEXTAREA_BRAND} from './z-textarea';

export {zColor} from './z-color.js';
export {zGoogleFont} from './z-google-font.js';
export {zTextarea} from './z-textarea.js';

export const ZodZypesInternals = {
Expand Down
7 changes: 7 additions & 0 deletions packages/zod-types/src/z-google-font.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import {getAvailableFonts} from '@remotion/google-fonts';

Check failure on line 1 in packages/zod-types/src/z-google-font.ts

View workflow job for this annotation

GitHub Actions / Linting + Formatting 

Cannot find module '@remotion/google-fonts' or its corresponding type declarations.
Copy link
Member

Choose a reason for hiding this comment

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

Thanks a lot for starting this initiative!
Unfortunately I think it's not a good idea to use @remotion/google-fonts as a dependency.

@remotion/zod-types is a lightweight package and many people use it to for example show a color picker.
Due to it's massive database, @remotion/google-fonts is a 50MB package that most people will not even need in this case.

For a zGoogleFont type, we should use the Google Fonts package and write a script to generate a list of available fonts, then use that instead of depending on the whole package.

Copy link
Author

@erayerdin erayerdin Sep 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Just out of curiosity and to clarify: Why do we even have to write a generator script here? I have checked packages/google-fonts/scripts and it does not seem like it fetches fonts from Google Fonts API. And, in this case, all we need (I presume) is to get the names of the fonts and that's all.

So, doesn't that mean I can just copy all FontName.ts files under packages/google-fonts/src into packages/zod-types/src/z-google-fonts? Or maybe a create a big packages/zod-types/src/z-google-fonts/fonts.ts and create something like:

const fonts: string[] = [
  // ...
  'Roboto',
  'Ubuntu',
  // ...
];

Copy link
Member

Choose a reason for hiding this comment

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

The generator script does only retrieve metadata: List of available fonts, weights, styles, unicode ranges and most importantly the URLs at which it should fetch the actual fonts. The metadata powers also the TypeScript type completion and the getInfo() APIs. Yes even this data is massive.

I would say yes, you can just copy all font names into an array. Then the developer can itself retrieve the metadata from that and load fonts.

import {z} from 'zod';

const fonts = getAvailableFonts();
const fontImportNames = fonts.map(({importName}) => importName);

Check failure on line 5 in packages/zod-types/src/z-google-font.ts

View workflow job for this annotation

GitHub Actions / Linting + Formatting 

Binding element 'importName' implicitly has an 'any' type.

export const zGoogleFont = () => z.enum(['None', ...fontImportNames]);
Loading
Loading