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

feat(web-fonts): add downloadLocally options #3723

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

onmax
Copy link

@onmax onmax commented Apr 14, 2024

🏗️ WIP

Solves #3646

How does it work?

Using remote font

It works as before. We just moved the logic of fetching the remote css to its own function: getRemoteCSS

Using local font

  1. It will try to read the file if it has been already generated using readFontCSS. If it has not been generated a placeholder will be return. The content of that function is the content that it is returned to the getCSS of the preflight of the preset.
  2. In the Vite Plugin and then mainly use the useLocalFont function:
    2.1 Download the remote CSS using getRemoteCSS
    2.2 Extract the urls
    2.3 Download the urls and save them locally
    2.4 Update the CSS file with the local paths
    2.5 Write the CSS file

TODO

  • Verify that this process makes sense
  • Make tests

Copy link

netlify bot commented Apr 14, 2024

Deploy Preview for unocss ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 1b2dacc
🔍 Latest deploy log https://app.netlify.com/sites/unocss/deploys/662425ea75419a0008d4e7e5
😎 Deploy Preview https://deploy-preview-3723--unocss.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@onmax
Copy link
Author

onmax commented Apr 14, 2024

@userquin would you mind checking that the code makes sense?

Is there anything missing other than support for svelte for example? Thank you!

preflights.push(await importUrl(url))
}

preflights.push(provider.getPreflight?.(fontsForProvider))
Copy link
Author

Choose a reason for hiding this comment

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

This is missing for downloadLocally

@userquin userquin self-assigned this Apr 20, 2024
@userquin
Copy link
Member

userquin commented Apr 20, 2024

My commit should fix lint and build, we need to review logic for local font folder:

  • ✔️ check nuxt package to override css folder in resolveConfig
  • ✔️ check astro package
  • ✔️ check sveltekit package: no changes required

The preset will not initialize the css folder, the vite plugin will check if configured, if missing then it will use viteConfig.publicDir. Nuxt will require some logic to include the default value, and SvelteKit will require to change it to use static folder (IIRC kit will configure publicDir properly, I need to confirm it => https://kit.svelte.dev/docs/configuration#files can be changed but we don't have access to kit config, we need a new option seems to be configured in config hook, check https://github.com/sveltejs/kit/blob/c5d1e67202b307c790a88c0a3bb437205686824a/packages/kit/src/exports/vite/index.js#L649 and https://github.com/sveltejs/kit/blob/c5d1e67202b307c790a88c0a3bb437205686824a/packages/kit/src/exports/vite/index.js#L672).

We also need to simplify the logic, we don't want to deal with public, assets and external folders: my suggestion is to use always <public>/unocss-fonts:

  • Vite/Astro/Nuxt: ${viteConf.publicDir}/unocss-fonts
  • Kit: static/unocss-fonts

Of course, we also need to check Next and Webpack.

We also need to review reload logic, what if the font is changed? (we should remove css and fonts and download them again)

@userquin
Copy link
Member

userquin commented Apr 20, 2024

We cannot use webfont preset in playground, it is client side only, we should add a new example and some tests.

(I mean, the local fonts will be ignored in the client runtime (we have also UnoCSS configuration in the playground), only used in playground build)

@userquin
Copy link
Member

userquin commented Apr 20, 2024

We also need to include preconnect and font in the entry point (index.html), rn not adding those head entries.

(this should go in another PR, it is a new feat for the preset)

downloadBasePath: nuxt.options.app.baseURL,
}
nuxt.options.css ??= []
nuxt.options.css.push(`~/${relative(dirname(nuxt.options.dir.public), downloadDir)}/${defaultFontCssFilename}`.replaceAll('\\', '/'))
Copy link
Member

@userquin userquin Apr 20, 2024

Choose a reason for hiding this comment

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

should use unshift?

@userquin
Copy link
Member

we'll need to include css and font folders: css should go to assets folders in Vite and styles folder in Astro and Nuxt (this way we can use some CSS processing optimizations, Astro for example excluding css styles in public folder => https://docs.astro.build/en/guides/styling/#load-a-static-stylesheet-via-link-tags)

@userquin
Copy link
Member

userquin commented Apr 20, 2024

@onmax can you merge main and resolve the conflicts in the kit example? I will not touch the PR until tmr.

done ✔️

@onmax
Copy link
Author

onmax commented Apr 26, 2024

Sorry, I was doing other stuff. Anything missing in the PR?

@userquin
Copy link
Member

Sorry, I was doing other stuff. Anything missing in the PR?

We need to add webpack and CLI.

We should review the logic: check nuxt google fonts and google-fonts-helper

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.

None yet

2 participants