Skip to content

Conversation

lindsaylevine
Copy link

@lindsaylevine lindsaylevine commented Mar 29, 2021

Fixes #63

questions:

  1. how will this behave with the pizzafox plugin? answer: works
  2. how can this be tested within the plugin, if at all? answer: mock/spy

@lindsaylevine lindsaylevine added the type: feature code contributing to the implementation of a feature and/or user facing functionality label Mar 29, 2021
@lindsaylevine
Copy link
Author

@ehmicky any specific thoughts on these questions in the PR desc? 🙏

how will this behave with the pizzafox plugin?
how can this be tested within the plugin, if at all?

@ehmicky
Copy link

ehmicky commented Apr 1, 2021

The cache utility deletes the destination file/directory before both saving and restoring. So I believe it should be idempotent, and running both netlify-plugin-cache-nextjs and the logic in this PR should work.

However, I would suggest manually testing it before merging this PR. The production behavior of the caching utility is different from local builds, so I would suggest testing both (or at least production). If it turned out to be incompatible, I would suggest:

  • Detecting whether netlify-plugin-cache-nextjs is used. This could be done by using require('netlify-plugin-cache-nextjs').
  • If detected, the logic of this PR could be a noop.

When it comes to testing it, I would suggest using a fixture directory then:

  • Delete .next/ if already exists (from running the test previously)
  • Create .next/ with a .next/dummy in it
  • Run the plugin
  • Delete .next/ and .next/dummy
  • Run the plugin again
  • Assert that .next/dummy exists
  • Delete .next/ and .next/dummy

@lindsaylevine
Copy link
Author

i manually tested. as for automated tests, i'm not sure how we'd "run the plugin" with the real valid utils.cache hook?

@ehmicky
Copy link

ehmicky commented Apr 12, 2021

The above was assuming the tests would be run using @netlify/build programmatically.

Another option would be to spy on the mocked utils.cache.* methods, for example using jest.spyOn(). This would be simpler to implement, but would also not catch every type of bug.

@lindsaylevine lindsaylevine merged commit 1c686e2 into main Apr 21, 2021
@lindsaylevine lindsaylevine deleted the ll/cache-utility branch April 21, 2021 05:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: feature code contributing to the implementation of a feature and/or user facing functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cache .next/cache/ between builds

2 participants