Add CLI option to ignore invalid functions #867
Conversation
The presence of invalid functions shouldn't force a build failure as a deployment can still be valid (and even correct as outlined in cloudflare#845) with invalid functions. This commit adds a `--force` cli option (bike-shedding welcomed) that allows users to opt to ignore invalid functions (and skip more checks in the future?). Closes cloudflare#845
🦋 Changeset detectedLatest commit: 9f9e501 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
🧪 Prereleases are available for testing 🧪 @cloudflare/next-on-pagesYou can install this latest build in your project with: npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/next-on-pages/runs/10579290331/npm-package-next-on-pages-867@cloudflare/eslint-plugin-next-on-pagesYou can install this latest build in your project with: npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/next-on-pages/runs/10579290331/npm-package-eslint-plugin-next-on-pages-867 |
| if (collectedFunctions.invalidFunctions.size > 0) { | ||
| if ( | ||
| collectedFunctions.invalidFunctions.size > 0 && | ||
| !opts.ignoreInvalidFunctions |
There was a problem hiding this comment.
my concern with this approach is that people may find and use --force because they get this error, without actually accounting for the routes properly... In your case, you want to host these routes elsewhere, but I imagine there'll be people who don't realise they need to actually specify these routes should execute somewhere else, and expect it to just work by forcing a build.
There was a problem hiding this comment.
Same. I don't enjoy the thought of forcing a build.
I worry that the alternative of trying to better evaluate the next.config to determine if the function is actually invalid may be fraught with difficulty (I've not looked too closely, so I'll defer to your judgement). Is this what you had in mind?
Would printing a warning instead of exiting be sufficient? So the errors aren't swept under the rug.
From a connotation perspective, I find force to be ok, but I'm happy to rename the argument as well
The presence of invalid functions shouldn't force a build failure as a deployment can still be valid (and even correct as outlined in #845) with invalid functions.
This commit adds a
--forcecli option (discussion welcomed!) that allows users to opt to ignore invalid functions (and skip more checks in the future?).Closes #845