Replies: 2 comments
-
I think handleLoadUtils is a good fit... |
Beta Was this translation helpful? Give feedback.
0 replies
-
Sorry I missed this before. I don’t honestly have any strong feelings about this. My 2¢:
|
Beta Was this translation helpful? Give feedback.
0 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
-
Before v24.6.0, both the
utilsScript
option and theloadUtils
static method accepted a string URL for the hosted utils module and then internally did a dynamic import to fetch it. In v24.6.0 we renamed theutilsScript
option toloadUtilsOnInit
and added support for specifying your own dynamic import of utils, which allows bundlers (like Webpack, Vite, or Parcel) to automatically create a separate utils bundle and load it asynchronously when needed.Since it's an easy change for everyone to specify their own dynamic import (it just means switching from
loadUtilsOnInit: "/path/to/utils.js"
toloadUtilsOnInit: () => import("/path/to/utils.js")
), in v25 I want to simplify bothloadUtilsOnInit
andloadUtils
to stop accepting URL strings, and only work with this explicit dynamic import approach, which simplifies the code, and the docs, and means we can get rid of the internal dynamic import, which seems to have caused lots of people problems.In the case of the
loadUtils
static method, you'd end up with something like this:(here, we don't technically need to wrap the import in a function as it will be executed immediately, but I figured it was better to keep it this way in terms of error handling - otherwise, you would have to wrap it in your own try/catch and do your own error handling each time, which is annoying)
I think it would make sense to rename the static method to something that more closely describes what it does - maybe
handleLoadUtils
? (or invokeLoadUtils/callLoadUtils/doLoadUtils/executeLoadUtils/processLoadUtils - I'm open to suggestions). This would have the added benefit of freeing up the nameloadUtils
which we could then use for the initialisation option instead, sinceloadUtilsOnInit
feels a bit clunky to me. It feels clunky because first of all, the "on init" part is confusing because you're putting this in the plugin initialisation code, so this is already in the initialisation stage, so why do you need "on init"? Also, there is another asynchronous query that runs in parallel,geoIpLookup
, and that doesn't have "on init" in the name, which is inconsistent.So we would end up with something like this for this initialisation option:
and this for the static method:
Does anyone have any feedback, thoughts, or better ideas for names? cc: @Mr0grog
Beta Was this translation helpful? Give feedback.
All reactions