Repository: gohugoio/hugo (language: go)

Issue:
Investigate .RenderString regression
See https://discourse.gohugo.io/t/multilingual-related-page-renderstring-regression-in-0-92/36555/16?u=bep


I'm pretty sure this came from https://github.com/gohugoio/hugo/pull/9342 -- but I'm also pretty sure that that PR surfaced an already existing bug re output formats and `.Translations`, so I think we need to understand and fix the real issue rather than reverting the above.

@ptgott 



Discussion / hints:
Thanks for the heads up. I’ll have time to look into this over the weekend and possibly today.

Fix patch:
diff --git a/hugolib/page.go b/hugolib/page.go
index 286d210752c..11b41e16920 100644
--- a/hugolib/page.go
+++ b/hugolib/page.go
@@ -16,15 +16,12 @@ package hugolib
 import (
 	"bytes"
 	"fmt"
-	"html/template"
 	"os"
 	"path"
 	"path/filepath"
 	"sort"
 	"strings"
 
-	"github.com/mitchellh/mapstructure"
-
 	"github.com/gohugoio/hugo/identity"
 
 	"github.com/gohugoio/hugo/markup/converter"
@@ -47,7 +44,6 @@ import (
 
 	"github.com/gohugoio/hugo/media"
 	"github.com/gohugoio/hugo/source"
-	"github.com/spf13/cast"
 
 	"github.com/gohugoio/hugo/common/collections"
 	"github.com/gohugoio/hugo/common/text"
@@ -593,64 +589,6 @@ var defaultRenderStringOpts = renderStringOpts{
 	Markup:  "", // Will inherit the page's value when not set.
 }
 
-func (p *pageState) RenderString(args ...interface{}) (template.HTML, error) {
-	if len(args) < 1 || len(args) > 2 {
-		return "", errors.New("want 1 or 2 arguments")
-	}
-
-	var s string
-	opts := defaultRenderStringOpts
-	sidx := 1
-
-	if len(args) == 1 {
-		sidx = 0
-	} else {
-		m, ok := args[0].(map[string]interface{})
-		if !ok {
-			return "", errors.New("first argument must be a map")
-		}
-
-		if err := mapstructure.WeakDecode(m, &opts); err != nil {
-			return "", errors.WithMessage(err, "failed to decode options")
-		}
-	}
-
-	var err error
-	s, err = cast.ToStringE(args[sidx])
-	if err != nil {
-		return "", err
-	}
-
-	if err = p.pageOutput.initRenderHooks(); err != nil {
-		return "", err
-	}
-
-	conv := p.getContentConverter()
-	if opts.Markup != "" && opts.Markup != p.m.markup {
-		var err error
-		// TODO(bep) consider cache
-		conv, err = p.m.newContentConverter(p, opts.Markup, nil)
-		if err != nil {
-			return "", p.wrapError(err)
-		}
-	}
-
-	c, err := p.pageOutput.cp.renderContentWithConverter(conv, []byte(s), false)
-	if err != nil {
-		return "", p.wrapError(err)
-	}
-
-	b := c.Bytes()
-
-	if opts.Display == "inline" {
-		// We may have to rethink this in the future when we get other
-		// renderers.
-		b = p.s.ContentSpec.TrimShortHTML(b)
-	}
-
-	return template.HTML(string(b)), nil
-}
-
 func (p *pageState) addDependency(dep identity.Provider) {
 	if !p.s.running() || p.pageOutput.cp == nil {
 		return
@@ -658,29 +596,6 @@ func (p *pageState) addDependency(dep identity.Provider) {
 	p.pageOutput.cp.dependencyTracker.Add(dep)
 }
 
-func (p *pageState) RenderWithTemplateInfo(info tpl.Info, layout ...string) (template.HTML, error) {
-	p.addDependency(info)
-	return p.Render(layout...)
-}
-
-func (p *pageState) Render(layout ...string) (template.HTML, error) {
-	templ, found, err := p.resolveTemplate(layout...)
-	if err != nil {
-		return "", p.wrapError(err)
-	}
-
-	if !found {
-		return "", nil
-	}
-
-	p.addDependency(templ.(tpl.Info))
-	res, err := executeToString(p.s.Tmpl(), templ, p)
-	if err != nil {
-		return "", p.wrapError(errors.Wrapf(err, "failed to execute template %q v", layout))
-	}
-	return template.HTML(res), nil
-}
-
 // wrapError adds some more context to the given error if possible/needed
 func (p *pageState) wrapError(err error) error {
 	if _, ok := err.(*herrors.ErrorWithFileContext); ok {
@@ -981,13 +896,16 @@ func (p *pageState) shiftToOutputFormat(isRenderingSite bool, idx int) error {
 		if lcp, ok := (p.pageOutput.ContentProvider.(*page.LazyContentProvider)); ok {
 			lcp.Reset()
 		} else {
-			p.pageOutput.ContentProvider = page.NewLazyContentProvider(func() (page.ContentProvider, error) {
+			lcp = page.NewLazyContentProvider(func() (page.OutputFormatContentProvider, error) {
 				cp, err := newPageContentOutput(p, p.pageOutput)
 				if err != nil {
 					return nil, err
 				}
 				return cp, nil
 			})
+			p.pageOutput.ContentProvider = lcp
+			p.pageOutput.TableOfContentsProvider = lcp
+			p.pageOutput.PageRenderProvider = lcp
 		}
 	}
 
diff --git a/hugolib/page__common.go b/hugolib/page__common.go
index e718721f7fc..ccef79a3fe4 100644
--- a/hugolib/page__common.go
+++ b/hugolib/page__common.go
@@ -64,7 +64,6 @@ type pageCommon struct {
 	maps.Scratcher
 	navigation.PageMenusProvider
 	page.AuthorProvider
-	page.PageRenderProvider
 	page.AlternativeOutputFormatsProvider
 	page.ChildCareProvider
 	page.FileProvider
diff --git a/hugolib/page__new.go b/hugolib/page__new.go
index 8c96d5014dd..17bdb30ff6f 100644
--- a/hugolib/page__new.go
+++ b/hugolib/page__new.go
@@ -86,7 +86,6 @@ func newPageBase(metaProvider *pageMeta) (*pageState, error) {
 	ps.Eqer = ps
 	ps.TranslationKeyProvider = ps
 	ps.ShortcodeInfoProvider = ps
-	ps.PageRenderProvider = ps
 	ps.AlternativeOutputFormatsProvider = ps
 
 	return ps, nil
diff --git a/hugolib/page__output.go b/hugolib/page__output.go
index 377e16df522..41332347797 100644
--- a/hugolib/page__output.go
+++ b/hugolib/page__output.go
@@ -14,7 +14,6 @@
 package hugolib
 
 import (
-	"github.com/gohugoio/hugo/markup/converter"
 	"github.com/gohugoio/hugo/output"
 	"github.com/gohugoio/hugo/resources/page"
 	"github.com/gohugoio/hugo/resources/resource"
@@ -59,6 +58,7 @@ func newPageOutput(
 		pagePerOutputProviders:  providers,
 		ContentProvider:         page.NopPage,
 		TableOfContentsProvider: page.NopPage,
+		PageRenderProvider:      page.NopPage,
 		render:                  render,
 		paginator:               pag,
 	}
@@ -84,73 +84,19 @@ type pageOutput struct {
 	pagePerOutputProviders
 	page.ContentProvider
 	page.TableOfContentsProvider
+	page.PageRenderProvider
 
 	// May be nil.
 	cp *pageContentOutput
 }
 
-func (o *pageOutput) initRenderHooks() error {
-	if o.cp == nil {
-		return nil
-	}
-
-	var initErr error
-
-	o.cp.renderHooks.init.Do(func() {
-		ps := o.cp.p
-
-		c := ps.getContentConverter()
-		if c == nil || !c.Supports(converter.FeatureRenderHooks) {
-			return
-		}
-
-		h, err := ps.createRenderHooks(o.f)
-		if err != nil {
-			initErr = err
-			return
-		}
-		o.cp.renderHooks.hooks = h
-
-		if !o.cp.renderHooksHaveVariants || h.IsZero() {
-			// Check if there is a different render hooks template
-			// for any of the other page output formats.
-			// If not, we can reuse this.
-			for _, po := range ps.pageOutputs {
-				if po.f.Name != o.f.Name {
-					h2, err := ps.createRenderHooks(po.f)
-					if err != nil {
-						initErr = err
-						return
-					}
-
-					if h2.IsZero() {
-						continue
-					}
-
-					if o.cp.renderHooks.hooks.IsZero() {
-						o.cp.renderHooks.hooks = h2
-					}
-
-					o.cp.renderHooksHaveVariants = !h2.Eq(o.cp.renderHooks.hooks)
-
-					if o.cp.renderHooksHaveVariants {
-						break
-					}
-
-				}
-			}
-		}
-	})
-
-	return initErr
-}
-
 func (p *pageOutput) initContentProvider(cp *pageContentOutput) {
 	if cp == nil {
 		return
 	}
 	p.ContentProvider = cp
 	p.TableOfContentsProvider = cp
+	p.PageRenderProvider = cp
 	p.cp = cp
 }
 
diff --git a/hugolib/page__per_output.go b/hugolib/page__per_output.go
index f59b5f9b545..bd4e35a5b0c 100644
--- a/hugolib/page__per_output.go
+++ b/hugolib/page__per_output.go
@@ -24,6 +24,9 @@ import (
 	"unicode/utf8"
 
 	"github.com/gohugoio/hugo/identity"
+	"github.com/mitchellh/mapstructure"
+	"github.com/pkg/errors"
+	"github.com/spf13/cast"
 
 	"github.com/gohugoio/hugo/markup/converter/hooks"
 
@@ -94,7 +97,7 @@ func newPageContentOutput(p *pageState, po *pageOutput) (*pageContentOutput, err
 			}
 		}()
 
-		if err := po.initRenderHooks(); err != nil {
+		if err := po.cp.initRenderHooks(); err != nil {
 			return err
 		}
 
@@ -349,6 +352,145 @@ func (p *pageContentOutput) WordCount() int {
 	return p.wordCount
 }
 
+func (p *pageContentOutput) RenderString(args ...interface{}) (template.HTML, error) {
+	if len(args) < 1 || len(args) > 2 {
+		return "", errors.New("want 1 or 2 arguments")
+	}
+
+	var s string
+	opts := defaultRenderStringOpts
+	sidx := 1
+
+	if len(args) == 1 {
+		sidx = 0
+	} else {
+		m, ok := args[0].(map[string]interface{})
+		if !ok {
+			return "", errors.New("first argument must be a map")
+		}
+
+		if err := mapstructure.WeakDecode(m, &opts); err != nil {
+			return "", errors.WithMessage(err, "failed to decode options")
+		}
+	}
+
+	var err error
+	s, err = cast.ToStringE(args[sidx])
+	if err != nil {
+		return "", err
+	}
+
+	if err = p.initRenderHooks(); err != nil {
+		return "", err
+	}
+
+	conv := p.p.getContentConverter()
+	if opts.Markup != "" && opts.Markup != p.p.m.markup {
+		var err error
+		// TODO(bep) consider cache
+		conv, err = p.p.m.newContentConverter(p.p, opts.Markup, nil)
+		if err != nil {
+			return "", p.p.wrapError(err)
+		}
+	}
+
+	c, err := p.renderContentWithConverter(conv, []byte(s), false)
+	if err != nil {
+		return "", p.p.wrapError(err)
+	}
+
+	b := c.Bytes()
+
+	if opts.Display == "inline" {
+		// We may have to rethink this in the future when we get other
+		// renderers.
+		b = p.p.s.ContentSpec.TrimShortHTML(b)
+	}
+
+	return template.HTML(string(b)), nil
+}
+
+func (p *pageContentOutput) RenderWithTemplateInfo(info tpl.Info, layout ...string) (template.HTML, error) {
+	p.p.addDependency(info)
+	return p.Render(layout...)
+}
+
+func (p *pageContentOutput) Render(layout ...string) (template.HTML, error) {
+	templ, found, err := p.p.resolveTemplate(layout...)
+	if err != nil {
+		return "", p.p.wrapError(err)
+	}
+
+	if !found {
+		return "", nil
+	}
+
+	p.p.addDependency(templ.(tpl.Info))
+
+	// Make sure to send the *pageState and not the *pageContentOutput to the template.
+	res, err := executeToString(p.p.s.Tmpl(), templ, p.p)
+	if err != nil {
+		return "", p.p.wrapError(errors.Wrapf(err, "failed to execute template %q v", layout))
+	}
+	return template.HTML(res), nil
+}
+
+func (p *pageContentOutput) initRenderHooks() error {
+	if p == nil {
+		return nil
+	}
+
+	var initErr error
+
+	p.renderHooks.init.Do(func() {
+		ps := p.p
+
+		c := ps.getContentConverter()
+		if c == nil || !c.Supports(converter.FeatureRenderHooks) {
+			return
+		}
+
+		h, err := ps.createRenderHooks(p.f)
+		if err != nil {
+			initErr = err
+			return
+		}
+		p.renderHooks.hooks = h
+
+		if !p.renderHooksHaveVariants || h.IsZero() {
+			// Check if there is a different render hooks template
+			// for any of the other page output formats.
+			// If not, we can reuse this.
+			for _, po := range ps.pageOutputs {
+				if po.f.Name != p.f.Name {
+					h2, err := ps.createRenderHooks(po.f)
+					if err != nil {
+						initErr = err
+						return
+					}
+
+					if h2.IsZero() {
+						continue
+					}
+
+					if p.renderHooks.hooks.IsZero() {
+						p.renderHooks.hooks = h2
+					}
+
+					p.renderHooksHaveVariants = !h2.Eq(p.renderHooks.hooks)
+
+					if p.renderHooksHaveVariants {
+						break
+					}
+
+				}
+			}
+		}
+	})
+
+	return initErr
+}
+
 func (p *pageContentOutput) setAutoSummary() error {
 	if p.p.source.hasSummaryDivider || p.p.m.summary != "" {
 		return nil
diff --git a/resources/page/page_lazy_contentprovider.go b/resources/page/page_lazy_contentprovider.go
index a513a063a16..206695d181a 100644
--- a/resources/page/page_lazy_contentprovider.go
+++ b/resources/page/page_lazy_contentprovider.go
@@ -19,6 +19,16 @@ import (
 	"github.com/gohugoio/hugo/lazy"
 )
 
+// OutputFormatContentProvider represents the method set that is "outputFormat aware" and that we
+// provide lazy initialization for in case they get invoked outside of their normal rendering context, e.g. via .Translations.
+// Note that this set is currently not complete, but should cover the most common use cases.
+// For the others, the implementation will be from the page.NoopPage.
+type OutputFormatContentProvider interface {
+	ContentProvider
+	TableOfContentsProvider
+	PageRenderProvider
+}
+
 // LazyContentProvider initializes itself when read. Each method of the
 // ContentProvider interface initializes a content provider and shares it
 // with other methods.
@@ -27,13 +37,13 @@ import (
 // will be needed. Must create via NewLazyContentProvider.
 type LazyContentProvider struct {
 	init *lazy.Init
-	cp   ContentProvider
+	cp   OutputFormatContentProvider
 }
 
 // NewLazyContentProvider returns a LazyContentProvider initialized with
 // function f. The resulting LazyContentProvider calls f in order to
 // retrieve a ContentProvider
-func NewLazyContentProvider(f func() (ContentProvider, error)) *LazyContentProvider {
+func NewLazyContentProvider(f func() (OutputFormatContentProvider, error)) *LazyContentProvider {
 	lcp := LazyContentProvider{
 		init: lazy.New(),
 		cp:   NopPage,
@@ -97,3 +107,18 @@ func (lcp *LazyContentProvider) Len() int {
 	lcp.init.Do()
 	return lcp.cp.Len()
 }
+
+func (lcp *LazyContentProvider) Render(layout ...string) (template.HTML, error) {
+	lcp.init.Do()
+	return lcp.cp.Render(layout...)
+}
+
+func (lcp *LazyContentProvider) RenderString(args ...interface{}) (template.HTML, error) {
+	lcp.init.Do()
+	return lcp.cp.RenderString(args...)
+}
+
+func (lcp *LazyContentProvider) TableOfContents() template.HTML {
+	lcp.init.Do()
+	return lcp.cp.TableOfContents()
+}


Test patch:
diff --git a/hugolib/page_test.go b/hugolib/page_test.go
index fc01bbf25eb..5e89278e1f4 100644
--- a/hugolib/page_test.go
+++ b/hugolib/page_test.go
@@ -768,6 +768,68 @@ Here is the last report for commits in the year 2016. It covers hrev50718-hrev50
 `)
 }
 
+// Issue 9383
+func TestRenderStringForRegularPageTranslations(t *testing.T) {
+	c := qt.New(t)
+	b := newTestSitesBuilder(t)
+	b.WithLogger(loggers.NewBasicLoggerForWriter(jwalterweatherman.LevelError, os.Stderr))
+
+	b.WithConfigFile("toml",
+		`baseurl = "https://example.org/"
+title = "My Site"
+
+defaultContentLanguage = "ru"
+defaultContentLanguageInSubdir = true
+
+[languages.ru]
+contentDir = 'content/ru'
+weight = 1
+
+[languages.en]
+weight = 2
+contentDir = 'content/en'
+
+[outputs]
+home = ["HTML", "JSON"]`)
+
+	b.WithTemplates("index.html", `
+{{- range .Site.Home.Translations -}}
+	<p>{{- .RenderString "foo" -}}</p>
+{{- end -}}
+{{- range .Site.Home.AllTranslations -}}
+	<p>{{- .RenderString "bar" -}}</p>
+{{- end -}}
+`, "_default/single.html",
+		`{{ .Content }}`,
+		"index.json",
+		`{"Title": "My Site"}`,
+	)
+
+	b.WithContent(
+		"ru/a.md",
+		"",
+		"en/a.md",
+		"",
+	)
+
+	err := b.BuildE(BuildCfg{})
+	c.Assert(err, qt.Equals, nil)
+
+	b.AssertFileContent("public/ru/index.html", `
+<p>foo</p>
+<p>foo</p>
+<p>bar</p>
+<p>bar</p>
+`)
+
+	b.AssertFileContent("public/en/index.html", `
+<p>foo</p>
+<p>foo</p>
+<p>bar</p>
+<p>bar</p>
+`)
+}
+
 // Issue 8919
 func TestContentProviderWithCustomOutputFormat(t *testing.T) {
 	b := newTestSitesBuilder(t)