Open SachaG opened 1 year ago
Quick feedback:
During build, performances are not a big deal, however I cannot yet load everything statically during build because Next doesn't support static layout (eg loading the survey definition) mixed with dynamic pages (eg loading the user response for the survey).
So, on some pages we may end up loading the survey on every request. I am not fond of using GraphQL because it's slower, and we do not benefit from GraphQL in this use case, since we have roughly 3 types of query (getting the survey definition, getting the question, and getting the list of surveys) that barely ever change. Calling Redis seems more efficient and simpler to me. I also use a bit of short-lived in-memory caching (eg a few minutes) to reduce the workload, since all users will load the same survey definitions there is no need to reload on each request.
I am not yet sure if we want to put this logic in API, maybe just in shared code instead? I think the API is more relevant for the result app which is highly dynamic than the surveyform we is simpler and can be optimized a bit more "aggressively" and thus need to stay independant.
for surveyId
and editionId
it should be done in surveyform, though there might be some old code left in some places. I haven't updated surveyadmin yet though
yes for response, I am currently working on the API endpoints to use Next 13.3 route handlers, so we can stop using GraphQL. For this to work, we must make sure that all responses have an editionId
. We might need to migrate older responses to have this field too.
So, on some pages we may end up loading the survey on every request. I am not fond of using GraphQL because it's slower, and we do not benefit from GraphQL in this use case, since we have roughly 3 types of query (getting the survey definition, getting the question, and getting the list of surveys) that barely ever change. Calling Redis seems more efficient and simpler to me.
Yes I agree, I just meant we should consider the API as the source of truth, but we can access a cached copy of its data, that's fine.
I also use a bit of short-lived in-memory caching (eg a few minutes) to reduce the workload, since all users will load the same survey definitions there is no need to reload on each request.
If that's possible then yes I think in-memory caching (nodeCache or similar) is the best solution. But I thought it wouldn't work for surveyform
because of the serverless architecture?
For the serverless architecture, that's a very good question and I was surprised by the answer:
You still have to make the code serverless-friendly, for instance each page should connect to the database independently from the others, because you can't tell which is requested first or which will actually get bundled. But in the end you will not have that many instances so caching works ok.
As I'm working on the codebase, a couple notes of stuff we could improve to discuss later.
.eslintrc
,.prettierrc
, etc. files to ensure every project follows the same formatting guidelines; and ideally also find a way to make sure every subdirectory uses a similartsconfig.json
const f = ({ arg1, arg2 }) => ...
instead ofconst f = (arg1, arg2) => ...
because it avoids any risk of messing up the order of the arguments. Maybe we can start applying that pattern throughout the whole monorepo?api2
project./surveyform
usessurveyId
andeditionId
when saving new responses.responses
document follow ajs2021__features__nullish_coalescing__experience
naming pattern due to the fact that we only had a single schema for all surveys, and wanted to avoid naming collisions. Could we drop this requirement and just have e.g.nullish_coalescing__experience
now? (at least we should get rid of the__experience
suffix which should be replaced by looking up the proper template object corresponding to the question)surveyadmin
a lot since we don't really need any of the Vulcan models/permissions/forms/etc. It would work better as a "normal" Next.js/Node app I think, closer toapi
but with the admin GUI part added.