Avoid loading validator un-necessarily#718
Avoid loading validator un-necessarily#718mcollina merged 2 commits intofastify:masterfrom brettwillis:patch-1
Conversation
The validator module pulls in the entire `ajv` dependency and several others and is not always used depending on the condition. Avoid loading this module in generated code when not necessary. Signed-off-by: Brett Willis <brettjohnwillis@gmail.com>
There was a problem hiding this comment.
lgtm, except the part where it requires dependencies from other places than the fast-json-stringify/lib/standalone module. It's easier for us to ship changes in the dependencies in that way. Can you keep all demendencies inside the fast-json-stringify/lib/standalone and export them dynamically instead?
From:
module.exports.dependencies = {
Serializer: require('./serializer'),
Validator: require('./validator')
}To:
module.exports.dependencies = {
get Serializer() { return require('./serializer') },
get Validator() { return require('./validator') }
}|
Ok, while yes that would still be a worthwhile improvement on the current state, it has the following drawbacks versus my initial suggestion
As you may gather I'm coming from the perspective of a bundled build and I'm concerned about the huge amount of dependency code that get's pulled in to the bundle just by this one line of generated code.
I don't fully understand the history/implications there but as we're in full control of the generated code, isn't it easy to control the dependencies that are imported? It seems like the dependencies are already nicely controlled and isolated in |
There was a problem hiding this comment.
I see. That makes sence for bundling.
I don't fully understand the history/implications there but as we're in full control of the generated code, isn't it easy to control the dependencies that are imported? It seems like the dependencies are already nicely controlled and isolated in ./lib/validator and ./lib/serializer.
The main issue here is that we don't require user to use the same lib version for standalone code generation and standalone code execution. This should work for all version under the same major. But anyway, it doesn't look like we've had a lot of changes in the standalone dependencies for a while, so I think we can change it.
lgtm after fixing the linting issue
The validator module pulls in the entire
ajvdependency and several others and is not always used depending on the condition. Avoid loading this module in generated code when not necessary.Checklist
npm run testandnpm run benchmarktests and/or benchmarks are includeddocumentation is changed or addedand the Code of conduct