refactor: replace dynamic imports with static imports #5821
Conversation
- Replace await import('path-to-module') with static imports
at the beginning of modules
There was a problem hiding this comment.
Looks generally good. Some imports were moved that mentioned performance reasons for having them as late-imports, we should try out if importing them statically makes a difference. @lukasholzer could you take a second look? You opened the original issue.
| @@ -259,9 +261,6 @@ const detectFrameworkSettings = async ({ projectDir }) => { | |||
| } | |||
|
|
|||
| if (frameworks.length > 1) { | |||
| // performance optimization, load inquirer on demand | |||
There was a problem hiding this comment.
if this was here as a performance optimisation, maybe it should stay as a dynamic import. @lukasholzer what do you think?
There was a problem hiding this comment.
TBH I would benchmark it and test it locally if it makes a difference. If not than we can move forward with a static one otherwise I would leave it a dynamic one.
| @@ -30,9 +31,6 @@ const buildFunction = async ({ cache, config, directory, func, hasTypeModule, pr | |||
| } | |||
| const functionDirectory = path.dirname(func.mainFile) | |||
|
|
|||
| // performance | |||
There was a problem hiding this comment.
this mentions performance as the reason we're late-importing it, so we should double-check wether we want to make it static.
| @@ -175,9 +176,6 @@ export class FunctionsRegistry { | |||
| // This function is here so we can mock it in tests | |||
| // eslint-disable-next-line class-methods-use-this | |||
| async listFunctions(...args) { | |||
| // Performance optimization: load '@netlify/zip-it-and-ship-it' on demand. | |||
| const { listFunctions } = await import('@netlify/zip-it-and-ship-it') | |||
There was a problem hiding this comment.
same, mentions perf optimisation
|
As previously stated I would do some benchmarks for every dynamic import and see how it influences the execution time. |
|
I ran a very naïve comparison, measuring the time between starting I ran on |
📊 Benchmark resultsComparing with 99da9eb
|
|
Thank you so much for the contribution @hereje! And please excuse the long idle time. |
🎉 Thanks for submitting a pull request! 🎉
Summary
Fixes #5697
Refactor dynamic imports with static imports on ESM (*.mjs) files
For us to review and ship your PR efficiently, please perform the following steps:
passes our tests.
A picture of a cute animal (not mandatory, but encouraged)