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

Templ formatting breaks imports at v0.2.747 #859

Closed
kanstantsin-chernik opened this issue Jul 24, 2024 · 8 comments
Closed

Templ formatting breaks imports at v0.2.747 #859

kanstantsin-chernik opened this issue Jul 24, 2024 · 8 comments

Comments

@kanstantsin-chernik
Copy link

kanstantsin-chernik commented Jul 24, 2024

There is a problem we noticed with formatting after upgrading to v0.2.747:

  • Some lines are duplicated with and without an alias

To Reproduce
To reproduce you need multiple lines with hyphen in the folder name but package name without like this
import "someproject/components/standard-metadata"

someproject/components/standard-metadata/some.go file has a standardmetadata package name. In go it is ok to import a package with hyphens without an alias.
templ fmt . adds the alias, which isn't a big deal, but breaking imports is.

Expected behavior
Either leave imports as is without adding the aliases or add the alias but not remove or duplicate lines

Logs
N/A

templ info output
Run templ info and include the output.

 % templ info
usage: templ <command> [<args>...]

templ - build HTML UIs with Go

See docs at https://templ.guide

commands:
  generate   Generates Go code from templ files
  fmt        Formats templ files
  lsp        Starts a language server for templ files
  version    Prints the version

% templ version
v0.2.747

Desktop (please complete the following information):

  • OS: Linux
  • templ CLI version (templ version): v0.2.747
  • Go version (go version): go1.22.5
  • gopls version (gopls version): v0.14.2

Additional context
Reproducing seems to be deterministic. Same input generates same broken output

@kanstantsin-chernik
Copy link
Author

Seems like this PR is at fault

@kanstantsin-chernik
Copy link
Author

kanstantsin-chernik commented Jul 24, 2024

Here is an example.
Input:

package somepackage

import (
	"code.uber.internal/delivery/eats-web-feed/mapper/css-class"
	twu "code.uber.internal/delivery/eats-web-feed/presentation/utils/tailwind-utils"
	tu "code.uber.internal/delivery/eats-web-feed/presentation/utils/templ-utils"
	richtextview "code.uber.internal/delivery/eats-web-feed/shared/components/base-idl/richtext"
	"code.uber.internal/delivery/eats-web-feed/shared/components/base-web-components/base-tag"
	"code.uber.internal/delivery/eats-web-feed/shared/components/quick-add-stepper"
	"thriftrw/code.uber.internal/eats/presentation/eater/mobile-web-shared/models/catalog_section"
)

templ View(input Input) {
	if input.DisplayType == catalog_section.SomeDisplayType {
		@richtextview.View(sometest.Model{})
	}
	<img class={ cssclass.SomeClasses(), templ.KV("some-class", input.ImageFit == twu.SomeFit) } alt="" { tu.LazyLoad(input.LazyLoadImage)... } src={ tu.ToString("balh") }/>
	@quickaddstepper.View()
	@basetag.View()
}

Output:

package somepackage

import (
	cssclass "code.uber.internal/delivery/eats-web-feed/mapper/css-class"
	twu "code.uber.internal/delivery/eats-web-feed/presentation/utils/tailwind-utils"
	tu "code.uber.internal/delivery/eats-web-feed/presentation/utils/templ-utils"
	richtextview "code.uber.internal/delivery/eats-web-feed/shared/components/base-idl/richtext"
	basetag "code.uber.internal/delivery/eats-web-feed/shared/components/base-web-components/base-tag"
	"code.uber.internal/delivery/eats-web-feed/shared/components/quick-add-stepper"
	quickaddstepper "code.uber.internal/delivery/eats-web-feed/shared/components/quick-add-stepper"
	"thriftrw/code.uber.internal/eats/presentation/eater/mobile-web-shared/models/catalog_section"
)

templ View(input Input) {
	if input.DisplayType == catalog_section.SomeDisplayType {
		@richtextview.View(sometest.Model{})
	}
	<img class={ cssclass.SomeClasses(), templ.KV("some-class", input.ImageFit == twu.SomeFit) } alt="" { tu.LazyLoad(input.LazyLoadImage)... } src={ tu.ToString("balh") }/>
	@quickaddstepper.View()
	@basetag.View()
}

As you can see, "code.uber.internal/delivery/eats-web-feed/shared/components/quick-add-stepper" is duplicated

@kanstantsin-chernik kanstantsin-chernik changed the title Templ formatting is broken at v0.2.747 Templ formatting breaks imports at v0.2.747 Jul 24, 2024
@a-h
Copy link
Owner

a-h commented Aug 19, 2024

Thanks for the bug report, and sorry for the inconvenience.

The tests for templ auto-import adjustments are in the repo at /cmd/templ/imports/testdata/. Each one is a .txtar file containing something like this.

The first section (delineated by the --) is the input, and the second section is the expected output.

-- fmt_templ.templ --
package test

import (
  sconv "strconv"
)

// Comment on variable or function.
var x = fmt.Sprintf(sconv.Quote("Hello"))
-- fmt_templ.templ --
package test

import (
	"fmt"
	sconv "strconv"
)

// Comment on variable or function.
var x = fmt.Sprintf(sconv.Quote("Hello"))

This makes it pretty easy to add new test cases, so I dropped your sample input into a file, and ran go test in /cmd/templ/imports.

I didn't see a duplicate quick-add-stepper in this test, but I did see css-class and base-tag get stripped out because the packages can't be found by the golang.org/x/tools/imports.Process function. The Process function uses the full filename to find the go.mod file etc. to locate packages.

                package somepackage                                                                                                                                                                                  
                                                                                                                                                                                                                     
                import (                                                                                                                                                                                             
            -           "code.uber.internal/delivery/eats-web-feed/mapper/css-class"
                        twu "code.uber.internal/delivery/eats-web-feed/presentation/utils/tailwind-utils"
                        tu "code.uber.internal/delivery/eats-web-feed/presentation/utils/templ-utils"
                        richtextview "code.uber.internal/delivery/eats-web-feed/shared/components/base-idl/richtext"
            -           "code.uber.internal/delivery/eats-web-feed/shared/components/base-web-components/base-tag"
                        "code.uber.internal/delivery/eats-web-feed/shared/components/quick-add-stepper"
                        "thriftrw/code.uber.internal/eats/presentation/eater/mobile-web-shared/models/catalog_section"

So, I wonder if the test can't reproduce this because of the lack of go.mod etc.

I'll try that next.

@a-h
Copy link
Owner

a-h commented Aug 19, 2024

Yes, I can reproduce with a simpler example.

go mod init github.com/a-h/templ/cmd/templ/imports/testdata/module
go get github.com/a-h/templ
mkdir css-classes

css-classes/classes.go

package cssclasses

const Header = "header"

main.templ

package main

import (
	"context"
	"github.com/a-h/templ/cmd/templ/imports/testdata/module/css-classes"
	"os"
)

templ View() {
	{ cssclasses.Header }
}

func main() {
	View().Render(context.Background(), os.Stdout)
}

Output

This is then formatted to:

package main

import (
	"context"
	"github.com/a-h/templ/cmd/templ/imports/testdata/module/css-classes"
	cssclasses "github.com/a-h/templ/cmd/templ/imports/testdata/module/css-classes"
	"os"
)

templ View() {
	{ cssclasses.Header }
}

func main() {
	View().Render(context.Background(), os.Stdout)
}

@a-h
Copy link
Owner

a-h commented Aug 19, 2024

OK, I have a PR for this at #890

If you have time to build templ from source and test it out, that would be great.

@a-h
Copy link
Owner

a-h commented Aug 19, 2024

Actually, might need a bit more thinking as per golang/go#28428 - the expected behaviour is to always add the alias.

But it shouldn't add two copies of the import and break stuff.

@a-h
Copy link
Owner

a-h commented Aug 24, 2024

Fixed in 4b2219c

@a-h a-h closed this as completed Aug 24, 2024
@kanstantsin-chernik
Copy link
Author

Hi, sorry it took me so long to get back to it. The issue is still there
Screenshot 2024-09-04 at 12 39 06 PM

Screenshot 2024-09-04 at 12 42 24 PM

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

No branches or pull requests

2 participants