Conversation
|
|
||
| // NewImportMapScriptElement creates a new ImportMapScriptElement. | ||
| // The data must be a valid JSON string. | ||
| func NewImportMapScriptElement(id string, data string) ImportMapScriptElement { |
There was a problem hiding this comment.
to stick with the convention in jsonscript.go, the signature of this would be ImportMapScript(...) ImportMapScriptElement.
also, there is no indication that this function panics in the signature. if a function might panic, it is usually best practice to name the function Must[original func name].
however, dealing with the panicing is probably a moot point as this logic should likely be handled in Render.
There was a problem hiding this comment.
I changed the function name as suggested.
There was a problem hiding this comment.
is this not just a special case of a JSONScript where type="importmap" and data is of type ImportMap? if so, we should simply reuse the logic here.
There was a problem hiding this comment.
The purpose of ImportMapScriptElement is to avoid Marshaling JSON data every render.
sylr
force-pushed
the
importmap
branch
4 times, most recently
from
8b755e7 to
0b13b72
Compare
ImportMapScriptElement renders a json importmap which allows to tell browsers how to resolve javascript dependencies. Signed-off-by: Sylvain Rabot <sylvain@abstraction.fr>
|
Your time and effort is much appreciated @sylr ! Thanks for putting time aside to contribute a feature. Though, we have in our guidelines an ask to first open a discussion when implementing a feature that hasn't already been discussed: https://github.com/a-h/templ/blob/main/CONTRIBUTING.md#come-up-with-a-design-and-share-it Definitely not saying that we won't merge your change, but it would bo good to just have a proposal issue in place so that we can ensure that it aligns with the goals of templ. Thanks again for contributing!! |
3 participants
ImportMapScriptElementrenders a json importmap which allows to tell browsers how to resolve javascript dependencies.