update failure points in plugin to do nothing instead #94
Conversation
lindsaylevine
added
type: chore
lindsaylevine
force-pushed
the
ll/update-failure-points
branch
from
1f2f3c2 to
21566bd
Compare
lindsaylevine
force-pushed
the
ll/update-failure-points
branch
from
21566bd to
5da6039
Compare
test/index.js
Outdated
| utils, | ||
| constants: { FUNCTIONS_SRC: 'out_functions' }, | ||
| }) | ||
| expect(output).toBe(undefined) |
There was a problem hiding this comment.
[sand] FYI, Jest has a toBeUndefined matcher.
index.js
Outdated
| @@ -82,6 +56,8 @@ module.exports = { | |||
| // inside `onPreBuild`. | |||
| const nextOnNetlify = require('next-on-netlify') | |||
| nextOnNetlify({ functionsDir: FUNCTIONS_SRC, publishDir: PUBLISH_DIR }) | |||
|
|
|||
| return true | |||
There was a problem hiding this comment.
Build plugin methods should not return values. Returning values is reserved for future features.
Also applies to onPreBuild().
There was a problem hiding this comment.
This is done per my suggestion to be able to test that the method skips artifacts generation based on return value.
Another option is indeed to verify that nothing was generated in the publish/functions directory (or remove those tests completely)
There was a problem hiding this comment.
It seems to me checking for the side effect of the function would give a stronger guarantee that it works as intended.
The problem with plugins returning values in methods is that they would prevent us from using return values in future features, which we might want to do.
lindsaylevine
force-pushed
the
ll/update-failure-points
branch
from
5da6039 to
f80b008
Compare
* update failure points in plugin to do nothing instead * refactor passing nextConfigPath into doesNotNeedPlugin
…onse tagging (#94) * feat: initial tags manifest wip * feat: augment prerender manifest * feat: refactor code * fix: tidy up * feat: async copying again * chore: consistency * chore: error check * chore: update test name for clarity * fix: move tag manifest * chore: remove unnecessary cache tag tests * fix: update page router cache tag prefix
Fixes #93
for the test cases that previously tested the failure points, i'm thinking i could just test that it doesnt copy the expected files over in onBuild()? curious if yall have a better idea for testing that the plugin does nothing lol