Skip to content

[javascript] Create @seleniumhq/atoms npm package from TypeScript atoms - #17459

Merged
AutomatedTester merged 3 commits into
trunkfrom
priceless-cannon-76a1b6
Oct 6, 2026
Merged

AutomatedTester merged 3 commits into
trunkfrom
priceless-cannon-76a1b6

Conversation

@AutomatedTester

@AutomatedTester AutomatedTester commented May 14, 2026 •

Copy link
Copy Markdown
Member

🔗 Related Issues

Builds on the Closure → TypeScript atom migrations (#17458 and siblings), which are already on trunk.

💥 What does this PR do?

Turns the three TypeScript atoms (get-attribute, find-elements, is-displayed) into real ES modules and compiles them down two independent paths from the same sources:

  • IIFE path (language bindings) — ts_project with --module commonjs, then a new cjs-to-iife.js post-processor emits browser-injectable (function(){ … return fn; })(). Python, Ruby, Java and .NET consumers see byte-compatible output and are unchanged.
  • npm path — ts_project with --module esnext, declaration, declarationMap and sourceMap into dist/, packaged as @seleniumhq/atoms with full .d.ts type declarations.

Both paths type-check the same .ts sources, so the published package and the injected atoms can't drift apart.

🔧 Implementation Notes

Atom sources

  • Removed the outer IIFE from get-attribute.ts, find-elements.ts and is-displayed.ts; each now ends in export default <fn>.
  • Added index.ts as the package entry point, re-exporting the three atoms as getAttribute, findElements and isDisplayed.

Build wiring (javascript/atoms/BUILD.bazel)

  • Replaced the three js_run_binary + tsc --module none --outFile invocations with ts_project targets.
  • Replaced the strip_trailing_semicolon binary with cjs_to_iife, which strips the CommonJS boilerplate tsc emits and wraps the remaining body in an IIFE returning the default export.
  • Updated the three wrap-*-as-global.js scripts to parse CJS-compiled input rather than asserting the file starts with (function () {.
  • Added atoms-npm-compiled (ts_project → dist/) and selenium-atoms-package (npm_package, publishable = True, tagged release-artifact).

One tsconfig per ts_project target
Each atom gets its own tsconfig-<atom>.json with a single-entry files list, rather than sharing one tsconfig-cjs.json across all three. A shared tsconfig makes several ts_project targets declare the same file as an input, which Bazel materialises concurrently into the same sandbox path — on Windows that races and fails with EPERM. Per-target configs give each action a disjoint input set. (tsconfig.json at the package root stays shared, because only atoms-npm-compiled uses it.)

Package metadata
javascript/atoms/package.json is promoted to @seleniumhq/atoms with exports entries for the package root and each individual atom.

✅ Verification

  • bazel build //javascript/atoms:selenium-atoms-package //javascript/atoms:atoms-npm-compiled plus the three *-typescript-compiled targets — clean.
  • bazel build //javascript/selenium-webdriver/lib/atoms:all //rb/lib/selenium/webdriver/atoms:all — clean; the generated IIFE output still ends in return get; })();, so the injection path is unchanged for bindings.

💡 Additional Considerations

  • package.json currently declares "main": "./dist/index.cjs" and a "require" condition on every export, but the npm path only emits ESM .js + .d.ts — no .cjs is produced. Either the CJS conditions should be dropped (ESM-only package) or a second ts_project should emit the .cjs outputs before this is published. Flagging for a decision before release.
  • The package version tracks the repo's nightly version string; it will need the usual release-tooling treatment the first time it ships.

🤖 AI assistance

  • No substantial AI assistance used
  • AI assisted (complete below)
    • Tool(s): Claude
    • What was generated: Templates
    • I reviewed all AI output and can explain the change

🔄 Types of changes

  • New feature (non-breaking change which adds functionality)
  • Cleanup (build wiring, formatting)

@selenium-ci selenium-ci added B-atoms JavaScript chunks generated by Google closure C-nodejs JavaScript Bindings B-build Includes scripting, bazel and CI integrations labels May 14, 2026
Refactors the three TypeScript atoms (get-attribute, find-elements,
is-displayed) from IIFE closures to proper ES modules with export
default, then produces two compilation targets from the same sources:

- IIFE path (language bindings): ts_project --module commonjs →
  cjs-to-iife.js → browser-injectable (function(){…return fn;})()
  so py/rb/java/dotnet consumers are unchanged.

- npm path: ts_project --module esnext + declaration → dist/ published
  as @seleniumhq/atoms with full TypeScript type declarations.

Key changes:
* Remove outer IIFE from all three atom .ts files; add export default.
* Add cjs-to-iife.js replacing strip-trailing-semicolon.js in the
  IIFE pipeline. Strips CJS boilerplate and wraps the body in an IIFE
  that returns the exported default function.
* Update wrap-as-global scripts to parse CJS-compiled input instead of
  asserting (function () at the start.
* Switch BUILD.bazel from js_run_binary+tsc(--module none --outFile)
  to ts_project for each atom, plus a new npm_package target.
* Add tsconfig.json (esnext, declaration) and tsconfig-cjs.json for
  the two compilation paths.
* Add index.ts as the package entry point.
* Promote javascript/atoms/package.json to @seleniumhq/atoms v4.45.x.
* Add get_attribute_typescript and find-elements-typescript CJS module
  targets to selenium-webdriver/lib/atoms/BUILD.bazel.

All 25 Python unit tests pass; //javascript/atoms:closure-test passes.
…roject targets

When multiple ts_project targets share a single tsconfig with no files/include
restriction, tsc compiles all .ts files for each target. On Windows, Bazel marks
outputs read-only after the first action completes; parallel actions then fail
with TS5033 EPERM trying to overwrite the same files.

Give each per-atom ts_project its own tsconfig that lists only the one .ts file
it is responsible for, eliminating the cross-target output collision.
@AutomatedTester
AutomatedTester force-pushed the priceless-cannon-76a1b6 branch from 0c73842 to 8146e8d Compare October 5, 2026 12:34
@AutomatedTester

Copy link
Copy Markdown
Member Author

npm packaging: changes needed before @seleniumhq/atoms can be published

Notes for whoever picks this up next. All references are against 8146e8da. The IIFE path for the language bindings looks fine — everything below is specific to the new npm artifact.

Blockers

1. Every require condition points at a file that is never built

javascript/atoms/package.json declares "main": "./dist/index.cjs" plus a require condition on all four exports entries (., ./get-attribute, ./find-elements, ./is-displayed). There is no CJS compilation on the branch. The only tsconfigs present are:

  • javascript/atoms/tsconfig.json — "module": "esnext", feeds :atoms-npm-compiled
  • javascript/atoms/typescript/tsconfig-{get-attribute,find-elements,is-displayed}.json — per-atom, feed the IIFE path

The tsconfig-cjs.json named in the first commit message is not in the tree — the second commit (giving each ts_project its own tsconfig to fix the Windows TS5033 collision) replaced it. So dist/index.cjs and dist/typescript/*.cjs are never emitted, and require('@seleniumhq/atoms') fails at resolution time for anyone who installs this.

Two ways out:

  • ESM-only (smaller, reasonable for a first release): drop "main" and all four require conditions. Add CJS later if a consumer asks.
  • Dual: add a second ts_project with a CJS tsconfig writing .cjs outputs, and add it to npm_package.srcs.

2. No LICENSE/NOTICE in the published package

npm_package.srcs is ["package.json", ":atoms-npm-compiled"]. Apache-2.0 §4(a) requires a copy of the License to travel with the distribution, and npm publish won't synthesise one.

javascript/selenium-webdriver/BUILD.bazel already has the pattern, including the gotcha in the comment:

# npm_package does not pick up filegroup sources.
genrule(
    name = "license",
    srcs = ["//:license"],
    outs = ["LICENSE", "NOTICE"],
    cmd = "cp $(locations //:license) $(@D)",
)

Add the equivalent in javascript/atoms/BUILD.bazel and list ":license" in npm_package.srcs.

Correctness

3. The types condition must come first

In all four exports entries types is listed after import/require. Export condition order is significant — resolvers take the first match. Reorder so each entry reads types, then import (then require, if dual). As written it only resolves by .d.ts adjacency rather than by the condition, which @arethetypeswrong/cli flags and which breaks under some moduleResolution settings.

4. Source maps reference files that aren't in the tarball

tsconfig.json sets "declarationMap": true and "sourceMap": true, but "files": ["dist/"] and npm_package.srcs both exclude the .ts sources. Every emitted .d.ts.map / .js.map points at ../typescript/*.ts, which is not published, so go-to-definition and stack mapping land on nothing.

Either add the sources to the package, or set both options to false.

5. The version is hardcoded and won't track releases

package.json pins "version": "4.45.0-nightly202605121853", and the npm_package target has no version attribute, so nothing rewrites it. For comparison, javascript/selenium-webdriver/BUILD.bazel defines VERSION = "4.51.0-nightly202609302012" and passes version = VERSION to its npm_package. Mirror that so the release tooling drives the version — as written this would publish six minors behind trunk.

Worth a look

6. "type": "module" has blast radius beyond the npm package

javascript/atoms/package.json is the only package.json in that subtree, so adding "type": "module" makes all 41 .js files under javascript/atoms/ ES modules as far as Node's resolver is concerned. That includes the four js_binary entry points, each of which opens with require('node:fs'):

  • typescript/cjs-to-iife.js
  • typescript/wrap-as-global.js
  • typescript/wrap-get-attribute-as-global.js
  • typescript/wrap-find-elements-as-global.js

The Bazel build passes, so rules_js evidently isn't staging the manifest next to them in the runfiles tree — but running any of these directly (node javascript/atoms/typescript/cjs-to-iife.js in.js out.js) now fails with ReferenceError: require is not defined in ES module scope, and the build's correctness ends up resting on a rules_js staging detail rather than on anything declared.

If #1 lands as ESM-only, "type": "module" can be dropped entirely and the module format declared through exports alone, which removes the hazard.

7. Leftover devDependencies

"google-closure-deps": "^20230802.0.0" is inherited from when this manifest was the private selenium-atoms build-tools package. It won't reach consumers, but it's now misleading. Confirm nothing in the closure dep-graph targets resolves it through this manifest before removing.

8. No README — npm_package.srcs includes none, so the npm page will render empty for a brand-new scope.

Out of band — not a code change

The @seleniumhq npm scope is currently empty: https://registry.npmjs.org/-/v1/search?text=scope:seleniumhq returns total: 0, and @seleniumhq/atoms 404s. The scope needs creating under the project's npm org, and the release token needs publish rights on it, before any publish job can succeed. Worth claiming regardless of when this PR lands, since an unused scope is squattable.

Verification

Once the above is addressed:

bazel build //javascript/atoms:selenium-atoms-package
bazel test //javascript/atoms:closure-test      # confirm the IIFE path is unaffected

# against the built package directory
npx @arethetypeswrong/cli --pack .
npm pack --dry-run      # confirm LICENSE present; no stray .ts unless maps are kept
node -e "import('@seleniumhq/atoms').then(m => console.log(Object.keys(m)))"

That last one should print [ 'findElements', 'getAttribute', 'isDisplayed' ].

The npm artifact was declared but not consumable. Addresses the
integration review on #17459:

- Drop the `require` conditions and `main` pointing at `dist/*.cjs`;
  no CJS compilation exists on the branch, so every `require` entry
  resolved to a file that is never built. Ship ESM-only and declare
  `engines.node >= 22.12.0`, where Node's `require(esm)` covers
  CommonJS consumers.
- Add `index.ts` extension specifiers. Extensionless specifiers are
  emitted verbatim under `module: esnext`, so `import('@seleniumhq/atoms')`
  failed with ERR_MODULE_NOT_FOUND for every consumer.
- Ship LICENSE/NOTICE, as Apache-2.0 4(a) requires.
- Put `types` first in each `exports` entry; condition order is
  significant and resolvers take the first match.
- Publish the `.ts` sources so the emitted `.d.ts.map` / `.js.map`
  resolve instead of pointing outside the tarball.
- Drive the version from `node:version` rather than a hardcoded value
  six minors behind trunk.
- Rename the four CommonJS build tools to `.cjs` so they no longer
  depend on `"type": "module"` not reaching them via runfiles staging.
- Add a consumer-facing `README.npm.md`, renamed to README.md on
  packaging so the contributor README stays in place.
- Drop the `google-closure-deps` devDependency left over from when this
  manifest was the private selenium-atoms build-tools package.

Verified by packing the built directory and installing it: both
`require` and `import` resolve, all four export paths resolve under
node10/node16/bundler per @arethetypeswrong/cli, and the IIFE path for
the language bindings is unchanged.
@AutomatedTester
AutomatedTester merged commit 4961c4f into trunk Oct 6, 2026
61 checks passed
@AutomatedTester
AutomatedTester deleted the priceless-cannon-76a1b6 branch October 6, 2026 19:04
AutomatedTester added a commit that referenced this pull request Oct 8, 2026
…8133)

The npm package landed in #17459 but nothing published it. Wire it into
the existing Node release path alongside selenium-webdriver.

* node:release publishes //javascript/atoms:selenium-atoms-package.publish
  before selenium-webdriver.publish. The "already published" rescue is
  extracted into publish_npm_package so both share it.
* node:verify checks both packages; node:build builds both.
* setup_github_npm_auth redirects the atoms registry for the nightly
  GitHub Packages path. The name is already scoped so it needs no rename,
  but the npmjs access flag is stripped — GitHub Packages derives
  visibility from the repository.
* Add publishConfig.access: public to javascript/atoms/package.json.
  Scoped packages default to restricted on npmjs, so without this the
  first publish is rejected.
* node:version now fails if the two versions have drifted. The bump works
  by substituting old_version, so drift would silently leave atoms stale
  and only surface as a node:verify failure after the release shipped.
* Convert the path/target literals to methods rather than constants.
  Rakefile loads this file twice (as node: and aliased as javascript:),
  which made constants warn on every invocation.

./go javascript:release dry-run passes end to end for both packages.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

B-atoms JavaScript chunks generated by Google closure B-build Includes scripting, bazel and CI integrations C-nodejs JavaScript Bindings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants