datadesk / baker

A build tool by and for the Los Angeles Times
MIT License
22 stars 3 forks source link

Make the dev watch work for createPages templates #1580

Closed brandonrobertz closed 8 months ago

brandonrobertz commented 8 months ago

This fixes the nunjucks engine watch so that it will rebuild the create pages on watch trigger. Without this fix, the dev server explodes with a template error as all of the context variables are undefined upon watch fire.

With this, I don't have to restart the dev server every time I make a change to a template file. It rebuilds every templated/createPage file in the baker config when it notices a change anywhere. (So it's could be slow on a very large number of create pages.)

I've been testing this (and using it on a project for ~6 months) and it appears to work well. There are still some issues with changing assets inside of the _data/ folder, but I'm not sure it's related to this or was an existing issue.

palewire commented 8 months ago

Thank you for this proposal. I'm going to give it a look.

What's the largest number of pages you've tested it on?

palewire commented 8 months ago

I quickly ran a manual examination in my local trunk. When I saved changes to the example/_layouts/object.njk template, the revisions appeared on pages like https://localhost:3000/object/this/ with a simple reload in my browser.

That is the expected behavior, correct?

brandonrobertz commented 8 months ago

I quickly ran a manual examination in my local trunk. When I saved changes to the example/_layouts/object.njk template, the revisions appeared on pages like https://localhost:3000/object/this/ with a simple reload in my browser.

That is the expected behavior, correct?

Yes! That's exactly what this PR is supposed to do.

brandonrobertz commented 8 months ago

Thank you for this proposal. I'm going to give it a look.

What's the largest number of pages you've tested it on?

The current interactive I'm working on generates just under 1k pages.

palewire commented 8 months ago

Thanks. I'm going to try to install your branch into some existing baker deployments with a larger number of dynamic pages, like these:

@vnessamartinez, do you have any concerns on this prior to merge?

vnessamartinez commented 8 months ago

Hi, @palewire. If it's working on the live pages you tested it on, sounds like it would be OK. Thank you for checking that! We're going to update our Baker version and can flag if we run into any issues with it when we use createPages soon.