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

Ignores generated font files on start/watch #695

Merged
merged 3 commits into from
Jun 25, 2021

Conversation

coreymcollins
Copy link
Contributor

@coreymcollins coreymcollins commented Jun 17, 2021

Closes #693

DESCRIPTION

This resolves (or attempts to resolves... test test test!) the issue of fonts disappearing ("randomly") when running start or watch. I was able to nail down the why – I'll copy my notes from the initial ticket below, but first I'll give the tldr; of everything.

We only need to add one line to clean-webpack-plugin to resolve this. After adding this, you can still add new fonts while running start or watch and they will end up in the build directory, but if you remove fonts from src/fonts while running start or watch, the removed font will remain in the build directory – until you stop your process and run a fresh npm run build.

The Cause

I can replicate the font erasure issue if I add an image to the src/images/ directory. Everything is fine if I add an SVG to src/images/svg, but if I add an image to (or remove an image from) the top-level /images/ directory, some of the fonts disappear.

On initial npm run build I wind up with my first screenshot above with basically duplicate font files – one named normally, then one with an string at the end.

While npm run start is running and I add an image, the extra string fonts are removed and the regular string ones remain.

I think this issue stems from Clean Webpack Plugin. We use that to clean our build directory so we don’t keep old/deleted files, and it works great and does its job. But, Tailwind wants to create font files itself and it does so with a string at the end like /raleway-medium-webfont.06ef4a38.woff2

It also changes the src in the built CSS file to match src: local("Raleway Medium"), url(fonts/raleway-medium-webfont.06ef4a38.woff2) format("woff2");

So when the Clean Webpack Plugin happens, it cleans our build directory (good), but Tailwind doesn’t regenerate its font files on npm run start (bad) which means we don’t get those fonts regenerated (bad) which means the built CSS file is now pointing to files that don’t exist (bad).

The Solution

For like a very iffy workaround, you can do this

new CleanWebpackPlugin( {
    cleanAfterEveryBuildPatterns: ['!fonts/**', '!*.woff2'],
} ),

This will ignore anything in build/fonts and any *.woff2 files in the build directory (just to be safe, I guess). This will still add new fonts to your build directory if you add them to your src while running npm run start, but it won’t remove anything from build if you remove it from src and won't actually create the Tailwind versions of the files. It'll just do the basic CopyWebpackPlugin thing of copying the file.

If you run a full npm run build, it seems like it generates all of the fonts that should be there (so far). Might not want to push your webpack config changes to a dev site or anywhere else, but may be a good workaround for locals since you shouldn’t really need to add too many new fonts mid-project anyway?

Still going to look into this some more.

Additional Notes

So, the above addition to CleanWebpackPlugin works with the following caveats:

  • Build fonts are only regenerated on npm run build
  • If you add or remove a font from the /src/fonts directory while running npm run start or npm run watch, nothing changes – fonts are not added or removed because of the cleanAfterEveryBuildPatterns ignore rule

I'm not sure if this is necessarily a blocker for our workflow? As I mentioned above, you generally aren't adding new fonts halfway through a project so you shouldn't have to be swapping fonts in and out all the time while running npm run start or anything in your daily flow.

Plus, knowing that the build directory does get cleaned and rebuilt (including /build/fonts means that fonts should always be doing what we expect them to be doing on lab/production since npm run build doesn't seem to be confined by the cleanAfterEveryBuildPatterns ignore rule.

I'll see about getting a PR spun up for this later this morning, and this will need extensive testing, but hopefully this solves the disappearing font issue or at least leads us down a path to solving it.

SCREENSHOTS

Tailwind-generated fonts
tailwind generated fonts

OTHER

  • Is this issue accessible? (Section 508/WCAG 2.0AA)
  • Does this issue pass all the linting? (PHPCS, ESLint, SassLint)
  • Does this pass CBT?

STEPS TO VERIFY

  1. Check out the branch
  2. npm ci
  3. Add fonts to your /src/fonts directory
  4. Add font face rules to a Sass partial somewhere (i.e., a new _fonts.scss partial)
  5. Add fontFamily values to Tailwind (see below)
  6. Run npm run build to see your fonts generated in the build directory
  7. Run npm run start and work like normal
  8. While running npm run start (or watch), add an image to the src/images/ directory. Previously, this would remove the Tailwind-generated fonts from the /build/ directory and break things. This should no longer happen.
  9. Play around adding/removing fonts while running start or watch to see how everything reacts
  10. Stop your process and run npm run build to see all of your fonts get built as expected

Per step 5 above, add something like this to your Tailwind config file (depending on the fonts you're adding, of course):

theme: {
	fontFamily: {
		sans: [ 'Raleway Regular', 'sans-serif' ],
		sansMedium: [ 'Raleway Medium', 'serif' ],
	},

By adding the fonts in the Tailwind config, we're ensuring that Tailwind will build the fonts. This means we don't need to add a rule to CopyWebpackPlugin to copy fonts the way we're doing with images and SVGs – Tailwind will just handle it.

DOCUMENTATION

Will this pull request require updating the wd_s wiki?

Yes, we'll need documentation for sure.

@coreymcollins
Copy link
Contributor Author

I've tagged @oliverharrison, @khleomix, and @itsamoreh as well as @gregrickaby for review here as they brought up the font disappearing issue on scrum and I want some folks who have actually run into this issue to test this in all of their use-cases.

Of course, having the whole team (and anyone else reading this) testing this would be great, too! The more the merrier.

Copy link
Contributor

@oliverharrison oliverharrison left a comment

Choose a reason for hiding this comment

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

Great work @coreymcollins !

@oliverharrison
Copy link
Contributor

I finally got some time to test this. Great work - this absolutely does fix things! Great work @coreymcollins! As you said - manually remove/add fonts, then stop your watch or start script. Running a build after that updates everything in the /build folder.

@oliverharrison oliverharrison merged commit f695b36 into main Jun 25, 2021
@oliverharrison oliverharrison deleted the fix/#693-presto-fonts-disappear branch June 25, 2021 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sometimes the fonts in /build will get deleted by npm
2 participants