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

text/template: add support for optional chaining #43608

Open
thernstig opened this issue Jan 9, 2021 · 9 comments
Open

text/template: add support for optional chaining #43608

thernstig opened this issue Jan 9, 2021 · 9 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@thernstig
Copy link

It would be beautiful if the package text/template (used by e.g. Helm) could support optional chaining. Similar to JavaScript in latest standard ECMAScript 2020. See https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Optional_chaining

It would basically look like this:

spec:
  type: {{ .Values.service?.type | default "ClusterIP" }}

If service is not defined, it would short-circuit to undefined/false and thus default to "ClusterIP".

This could be used in other cases, such as:

spec:
  {{- if .Values.global?.internalIPFamily }}
  ipFamily: {{ .Values.global.internalIPFamily }}
  {{- end }}

It solves a few things in e.g. Helm that uses text/template for templating:

  1. default not working when parent values does not exist in a chain, giving errors, see nil pointer evaluating interface when upper level doesn't exist prevents usage of default function helm/helm#8026.
  2. It would be able to get rid of the, in my opinion, not great best practice for nested or flat values for e.g. Helm (which I assume is applicable to other text/template usages as well), see https://helm.sh/docs/chart_best_practices/values/#flat-or-nested-values. Optional chaining would instead still make the code more readable (one-liner) not caring about value depth. It would also in my opinion make it easier to read YAML files with the nested structure, clearly separating levels, than to use camelCase.

As I know very little about Go and the packge text/template, feel free to add more precise info on how this translates to what needs to be done in text/template.

@cagedmantis cagedmantis changed the title Feature: Support for optional chaining in package text/template test/template: add support for optional chaining Jan 11, 2021
@cagedmantis cagedmantis changed the title test/template: add support for optional chaining text/template: add support for optional chaining Jan 11, 2021
@cagedmantis cagedmantis added this to the Backlog milestone Jan 13, 2021
@cagedmantis cagedmantis added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jan 13, 2021
@cagedmantis
Copy link
Contributor

This seems more like a proposal. I'm adding the text/template owners in order to begin the discussion.

/cc @robpike @mvdan

@bilak
Copy link

bilak commented Mar 18, 2021

@cagedmantis @robphoenix @mvdan please any update on this task? It would be super helpful if we can use this style of property resolver.

@thernstig
Copy link
Author

@bilak @cagedmantis @robphoenix @mvdan do you have some opinions on this?

@mvdan
Copy link
Member

mvdan commented Apr 27, 2021

There are dozens of open text/template issues. Please don't ping proposals every few weeks; we have limited time.

Personally, I think we should implement the already-accepted proposals (#20531 and #31103) before we consider new ones. Any help with those is welcome.

@thernstig
Copy link
Author

It was 5 months ago this was written, and I find it common courtesy to give a short comment that it has been seen. But each repo to his own.

@emmaLP

This comment was marked as duplicate.

@ianlancetaylor
Copy link
Contributor

The way to make progress on this is to turn it into a proposal with a precise definition of the suggested change and the new semantics.

For example, if I pass

var s struct {
    p *struct {
        f int
    }
}

to a template and write s.p.f then assuming that s.p is not nil I will get an integer. What should happen for s.p.?f if p is nil? Should I get 0? If I get nil that may cause a different error.

The initial comment on the issue says "feel free to add more precise info on how this translates to what needs to be done in text/template" but that is hard for us to do. There are many many many more people suggesting changes than there are people implementing those changes. This can only scale if the people interested in the change can describe exactly what how it should work.

Thanks.

@thernstig
Copy link
Author

@ianlancetaylor I do not have the knowledge in Go to give specifics, but regarding semantics one can look at https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Optional_chaining which can serve as the spec for how it should work.

But regarding s.p?.f it should return nil (since undefined does not exist in Go?).

Anyone more comfortable in Go; feel free to write up a proper proposal. I can link it in the original issue.

@ungerik
Copy link

ungerik commented Sep 26, 2022

There is no value in nil pointer panics and maybe not much code that depends on it to work properly. Because it just breaks things and doesn't fix anything, the nicest solution would be to make optional chaining the default.

Now I know we can't have nice things because of the remote chance that some code depends on the panics, backward compatibility and all, so the next best thing would be to make it a configuration option on template level.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

7 participants