Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add New NewFromTemplate Constructor for Parsed Templates #27

Merged
merged 5 commits into from
Dec 9, 2024

Conversation

ge3224
Copy link
Contributor

@ge3224 ge3224 commented Dec 6, 2024

This PR addresses the feature request to enhance the Gonertia library with a new constructor that supports initializing an Inertia instance using a parsed *template.Template.

Problem

Currently, the library requires a rootTemplateHTML field to create an Inertia instance, which limits flexibility for projects that already use parsed templates.

Solution

A new constructor method is introduced:

func NewFromTemplate(t *template.Template, opts ...Option) (*Inertia, error)

This allows direct initialization with a parsed template, leveraging the existing rootTemplate logic in the doHTMLResponse method.

Changes Made

  • Added the NewFromTemplate constructor.
  • Updated logic to ensure compatibility with the existing codebase.
  • Wrote unit tests to validate the new functionality.

Benefits

  • Integrates smoothly with existing template parsing workflows.
  • Reduces redundant template parsing for projects with pre-parsed templates.
  • Provides a cleaner and more flexible API for initializing Inertia.

Example

tmpl := template.Must(template.ParseFiles("layout.html", "components.html"))
inertia, err := NewFromTemplate(tmpl, 
    WithSharedProps(props),
    WithVersion("1.0")
)

References

Next Steps

Feedback on the implementation and additional edge cases is welcome. I am happy to address any review comments to ensure this aligns with the library's standards.

@@ -84,6 +84,31 @@ func NewFromBytes(rootTemplateBs []byte, opts ...Option) (*Inertia, error) {
return New(string(rootTemplateBs), opts...)
}

// NewFromTemplate receives a *template.Template and then initializes Inertia.
func NewFromTemplate(rootTemplate *template.Template, opts ...Option) (*Inertia, error) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just think maybe now we should refuse of lazy template parsing?
I mean, we could remove the buildRootTemplate from the doHTMLResponse and move it to the New constructor?
In turn, the New constructor only parse the template from string and pass it to our new function NewFromTemplate.
This will eliminate duplication of initialization logic in constructors and it will immediately return an error if something is wrong in the template.

Copy link
Owner

@romsar romsar Dec 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can also remove rootTemplateHTML from an Inertia struct then

Copy link
Contributor Author

@ge3224 ge3224 Dec 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we refuse lazy template parsing, would we need to continue to support the ShareTemplateFunc method? This method has to be run before template parsing.

Copy link
Owner

@romsar romsar Dec 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, you are right, we can't do that :(
I will review your pull request soon and we will merge it.

Copy link
Contributor Author

@ge3224 ge3224 Dec 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For my use case, users will need to ensure that Template.Funcs are defined before using the NewFromTemplate constructor. Perhaps we should alert them by returning an error if ShareTemplateFunc is called when no root template HTML is present?

// ShareTemplateFunc adds the passed value to the shared template func map. If
// no root template HTML string has been defined, it returns an error.
func (i *Inertia) ShareTemplateFunc(key string, val any) error {
	if i.rootTemplateHTML == "" {
		return fmt.Errorf("undefined root template html string")
	}

	i.sharedTemplateFuncs[key] = val
	return nil
}

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's right and we can merge it now, but maybe in the future we should abandon the shateTemplateFunc function and accept template functions in the constructor.

@ge3224 ge3224 force-pushed the new-from-go-template branch from 08a27c8 to ecab05d Compare December 7, 2024 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants