Skip to content

ci: run gulp bundle on Node 20 - #53

Merged
Tushar-TG-14 merged 5 commits into
tigergraph:mainfrom
ngarakapati:ci/fix-bundle-node16
Sep 16, 2026
Merged

ci: run gulp bundle on Node 20#53
Tushar-TG-14 merged 5 commits into
tigergraph:mainfrom
ngarakapati:ci/fix-bundle-node16

Conversation

@ngarakapati

@ngarakapati ngarakapati commented Sep 11, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Why

#52 made CI run gulp bundle. That failed on Node 20 because gulp-stylelint 13 / postcss-syntax still import postcss/lib/input. Node 17+ no longer allows that.

Pinning Node 16 worked around it, then lint showed real issues in theme-cloud.css. We can stay on Node 20 instead.

Fix

  • CI and .nvmrc use Node 20
  • Replace gulp-stylelint 13 with stylelint 14 (no postcss-syntax)
  • Keep existing CSS naming (disable the extra stylelint 14 pattern rules)
  • Same theme-cloud.css formatting/duplicate-selector fixes so lint can pass

Docs sites are unchanged. They still consume the zip, not Node.

Test plan

  • Confirm Actions uses Node 20
  • Confirm npx gulp bundle passes lint:css and writes the zip artifact
  • After merge, confirm main can publish the zip and ping Netlify once

@ngarakapati
ci: run gulp bundle on Node 16
05bed5b 
PostCSS 8.1 and gulp-stylelint 13 still use the ./ package-exports fallback. Node 17+ removed that, so lint:css crashed on GitHub's Node 20. Pin CI to Node 16 so the bundle job can finish.
@ngarakapati ngarakapati mentioned this pull request Sep 11, 2026
Merged
7 tasks
@ngarakapati
fix(ui): make theme-cloud.css pass gulp stylelint
45b39f3 
Node 16 got CI past the PostCSS crash. gulp bundle still failed lint:css on duplicate selectors and formatting in the Savanna theme. Merge those rules and keep Arial as a font name.
@ngarakapati
ci: keep Node 20 by moving CSS lint to stylelint 14
d6dcd37 
gulp-stylelint 13 cannot load PostCSS internals on Node 17+. Run stylelint 14 directly so CI can stay on Node 20. Keep the old CSS naming rules so existing stylesheets do not need a rewrite.
@ngarakapati ngarakapati changed the title ci: run gulp bundle on Node 16 ci: run gulp bundle on Node 20 Sep 11, 2026
@ngarakapati

ngarakapati commented Sep 11, 2026

Copy link
Copy Markdown
Contributor Author

@Tushar-TG-14 Please review

@ngarakapati
ci: use Node 24-compatible GitHub Actions
601e7ac 
checkout, setup-node, and upload-artifact v4 still target Node 20 on the runner, which GitHub now warns about. Bump those actions to v5. gulp still uses Node 20 from .nvmrc.
@ngarakapati
ci: bump upload-artifact to v6
d91d4ca 
v5 still declares Node 20 on the runner, which is why the warning remained after the v5 bump. v6 is the release that runs on Node 24 by default.
@Tushar-TG-14

Tushar-TG-14 commented Sep 16, 2026

Copy link
Copy Markdown
Collaborator

@ngarakapati

  1. I see a few changes in theme-cloud.css along with the Stylelint upgrade. Can you confirm these are only lint/formatting fixes and that there is no intended visual or behavioral change to the Savanna/Classic UI?
  2. Several Stylelint rules are now disabled. Were these added based on actual lint errors from the existing CSS? Just want to make sure we're disabling only the rules needed for compatibility.
  3. Can you confirm the full npx gulp bundle passes on Node 20, including CSS/JS lint, and successfully generates build/ui-bundle.zip?
  4. Since .nvmrc is changing from Node 10 to Node 20, can you also confirm the normal local gulp preview/build workflow works on Node 20?

@ngarakapati

ngarakapati commented Sep 16, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@Tushar-TG-14

  1. Yes. The theme-cloud.css edits are lint and formatting only. Duplicate selectors got merged, a calc() got wrapped, and a second .admonitionblock .paragraph { margin: 0 } was dropped because that rule is already earlier in the file. Nothing is meant to change how Savanna or Classic looks.

  2. Yes. Those extra Stylelint 14 rules came from actual lint failures on the CSS we already have (class names, custom properties, vendor prefixes, color notation). I turned off only what was blocking the upgrade. comment-empty-line-before and no-descending-specificity were already off. value-keyword-case is still on, with Arial and currentColor ignored.

  3. Yes. CI on this PR ran Node 20.20.2, npx gulp bundle passed lint:css and lint:js, and it uploaded build/ui-bundle.zip. Run: https://github.com/tigergraph/antora-ui/actions/runs/34600567189

  4. .nvmrc is 20 now, so local Node matches CI. gulp bundle is the same command CI runs. I have not re-run gulp preview as part of writing this. I can do that if you want it confirmed before merge.

@Tushar-TG-14
Tushar-TG-14 merged commit 4d89a4f into tigergraph:main Sep 16, 2026
1 check passed
@ngarakapati ngarakapati mentioned this pull request Sep 16, 2026
Merged
Tushar-TG-14 pushed a commit that referenced this pull request Sep 16, 2026
@ngarakapati
ci: stop committing the UI bundle from CI; contributors commit it
6228140 
The push-to-main step failed on every merge: it rebuilt build/ui-bundle.zip
and pushed it straight to main as github-actions[bot], which the branch
protection on main rejects (require a pull request, and require the build
check that a direct push can never have). See the failed run on the #53 merge.

Instead of fighting protection with tokens or a bot bypass, drop the
auto-commit entirely. Contributors rebuild the bundle locally with
`npx gulp bundle` and commit build/ui-bundle.zip in their PR, so it rides in
through the normal reviewed, checked path like any other change.

CI now only:
- on PRs: runs `npx gulp bundle` (lint + build) as the required `build` check,
- on merge to main: pings Netlify so docs sites pull the committed bundle.

No bot pushes to main, so no more protected-branch failures, no binary commits
created by CI, and no token or org-ruleset change needed. Permissions drop to
contents: read. The consumer URL (blob/main/build/ui-bundle.zip) is unchanged.
@ngarakapati
ngarakapati deleted the ci/fix-bundle-node16 branch September 16, 2026 19:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ngarakapati @Tushar-TG-14