Skip to content

fix(internal/librarian/nodejs): remove Format step #4792

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.

Sign up for GitHub

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

Merged
julieqiu merged 1 commit into from
Mar 24, 2026
Merged

fix(internal/librarian/nodejs): remove Format step #4792

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions internal/librarian/generate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,9 +204,6 @@ func generateLibraries(ctx context.Context, cfg *config.Config, libraries []*con
if err := nodejs.Generate(gctx, cfg, library, src); err != nil {
return fmt.Errorf("generate library %q (%s): %w", library.Name, cfg.Language, err)
}
if err := nodejs.Format(gctx, library); err != nil {
return fmt.Errorf("format library %q (%s): %w", library.Name, cfg.Language, err)
}
return nil
})
}
Expand Down
29 changes: 0 additions & 29 deletions internal/librarian/nodejs/generate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -465,35 +465,6 @@ func copySamplesFromStaging(stagingDir, outDir string) error {
return nil
}

// Format runs gts (npm run fix) on the library directory.
func Format(ctx context.Context, library *config.Library) error {
if err := ctx.Err(); err != nil {
return err
}

// ESLint exit codes:
// 0: No issues found.
// 1: Lint issues found (warnings or unfixable errors).
// 2: Configuration or fatal error.
//
// Exit code 1 is tolerated because generated code may contain expected,
// unfixable warnings (e.g., @typescript-eslint/no-explicit-any).
err := command.RunInDir(ctx, library.Output, "eslint",
"--fix",
"--ignore-pattern", "node_modules/",
"--no-error-on-unmatched-pattern",
"src/**/*.ts", "src/**/*.js")

if err != nil {
var exitErr *exec.ExitError
if errors.As(err, &exitErr) && exitErr.ExitCode() == 1 {
return nil
}
return fmt.Errorf("eslint failed: %w", err)
}
return nil
}

// DerivePackageName returns the npm package name for a library. It uses
// nodejs.package_name if set, otherwise derives it by splitting the library
// name on the second dash (e.g. "google-cloud-batch" → "@google-cloud/batch").
Expand Down
95 changes: 0 additions & 95 deletions internal/librarian/nodejs/generate_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
package nodejs

import (
"context"
"encoding/json"
"errors"
"fmt"
Expand Down Expand Up @@ -535,45 +534,6 @@ func TestRunPostProcessor_CustomScripts(t *testing.T) {
}
}

func TestFormat(t *testing.T) {
testhelper.RequireCommand(t, "eslint")
outDir := t.TempDir()
srcDir := filepath.Join(outDir, "src")
if err := os.MkdirAll(srcDir, 0755); err != nil {
t.Fatal(err)
}
eslintConfig := `{
"rules": {
"semi": ["error", "always"]
},
"parserOptions": {
"ecmaVersion": 2020,
"sourceType": "module"
}
}`
if err := os.WriteFile(filepath.Join(outDir, ".eslintrc.json"), []byte(eslintConfig), 0644); err != nil {
t.Fatal(err)
}
testFile := filepath.Join(srcDir, "index.ts")
if err := os.WriteFile(testFile, []byte("export const foo = 'bar'"), 0644); err != nil {
t.Fatal(err)
}
library := &config.Library{
Name: "google-cloud-test",
Output: outDir,
}
if err := Format(t.Context(), library); err != nil {
t.Fatal(err)
}
got, err := os.ReadFile(testFile)
if err != nil {
t.Fatal(err)
}
if !strings.Contains(string(got), "bar';") {
t.Errorf("expected fixed content with semicolon, got: %q", string(got))
}
}

func TestRunPostProcessor_PreservesFiles(t *testing.T) {
testhelper.RequireCommand(t, "gapic-node-processing")
testhelper.RequireCommand(t, "compileProtos")
Expand Down Expand Up @@ -892,61 +852,6 @@ func TestCopySamplesFromStaging_NonExistentDir(t *testing.T) {
}
}

func TestFormat_CanceledContext(t *testing.T) {
ctx, cancel := context.WithCancel(t.Context())
cancel()

library := &config.Library{
Name: "google-cloud-test",
Output: t.TempDir(),
}
err := Format(ctx, library)
if err == nil {
t.Fatal("expected error from canceled context, got nil")
}
if !errors.Is(err, context.Canceled) {
t.Errorf("expected context.Canceled, got: %v", err)
}
}

func TestFormat_ExitCode1(t *testing.T) {
testhelper.RequireCommand(t, "eslint")

outDir := t.TempDir()
srcDir := filepath.Join(outDir, "src")
if err := os.MkdirAll(srcDir, 0755); err != nil {
t.Fatal(err)
}

// Configure eslint with an "error" rule that cannot be auto-fixed.
// eslint exits 1 when unfixable errors remain after --fix.
eslintConfig := `{
"rules": {
"no-eval": "error"
},
"parserOptions": {
"ecmaVersion": 2020,
"sourceType": "module"
}
}`
if err := os.WriteFile(filepath.Join(outDir, ".eslintrc.json"), []byte(eslintConfig), 0644); err != nil {
t.Fatal(err)
}
// eval() triggers no-eval warning which --fix cannot remove.
if err := os.WriteFile(filepath.Join(srcDir, "index.js"), []byte("eval('1+1');\n"), 0644); err != nil {
t.Fatal(err)
}

library := &config.Library{
Name: "google-cloud-test",
Output: outDir,
}
// Format should tolerate exit code 1 (lint warnings).
if err := Format(t.Context(), library); err != nil {
t.Fatal(err)
}
}

func TestGenerateAPI_NoProtos(t *testing.T) {
googleapisDir := t.TempDir()
repoRoot := t.TempDir()
Expand Down
Loading