👷 Add API surface linting for @datadog/js-core via API Extractor#4804
Conversation
|
Bundles Sizes Evolution
|
thomas-lebeau
force-pushed
the
worktree-squishy-churning-salamander
branch
from
ad2dfaa to
30d3ba3
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c297c05069
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const name = path.basename(entryPoint, '.d.ts') | ||
| printLog(`Checking API surface for @datadog/js-core/${name}...`) | ||
|
|
||
| const configObject: IConfigFile = { |
There was a problem hiding this comment.
When GitLab runs check-js-core-api, this config uses API Extractor's default message reporting with localBuild: false; API Extractor documents ae-missing-release-tag as required by default, and non-local invocations fail when warnings are present. The current js-core declarations have JSDoc but no @public/@internal release tags, so yarn api:check will fail even when the committed reports match; add release tags to the exported API or suppress this message in the generated config before wiring the job into CI. See https://api-extractor.com/pages/messages/ae-missing-release-tag/.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
hmm, that does not seem to be the case, the tag are present in the golden reports and the checks passes
| }) | ||
|
|
||
| const packageDir = path.resolve('packages/js-core') | ||
| const entryPoints = globSync('cjs/entries/*.d.ts', { cwd: packageDir }) |
There was a problem hiding this comment.
Because the script discovers entry points from built cjs/entries/*.d.ts, it does not validate the actual subpaths exposed in packages/js-core/package.json. A change can remove ./monitor from the package exports while leaving src/entries/monitor.ts in place; the build will still emit the same declaration file and this loop will pass, even though modern consumers can no longer import @datadog/js-core/monitor. Use the package exports map (or compare it against the reports) as the source of truth for checked subpaths.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: acbdb016a6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const subpaths = Object.keys(packageJson.exports ?? {}).map((subpath) => subpath.replace('./', '')) | ||
|
|
||
| for (const name of subpaths) { | ||
| const entryPoint = path.join(packageDir, `cjs/entries/${name}.d.ts`) |
There was a problem hiding this comment.
issue: In package.json exports, the types condition is what TypeScript consumers resolve, but this loop derives only the subpath name and then always checks cjs/entries/${name}.d.ts. If someone changes or drops exports['./monitor'].types while leaving the built entry file in place, yarn api:check still passes even though @datadog/js-core/monitor exposes different or no declarations to node16/bundler users. Fresh evidence: this version now reads export keys, but this line still ignores each export value's types path; use that target and validate it exists as the API Extractor entry point.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
I think that's fine, let's not over do it, we can fix this if it's ever needed
Remove the package-specific tsconfig.api.json from @datadog/js-core and update the API Extractor script to use the root tsconfig.json instead.
- Add assembly.api.md golden file for the assembly entry point - Add assembly entry point to typedoc.json - Update util.api.md to include newly exported utilities: combine, deepClone, getType, isIndexableObject, mergeInto, RecursivePartial
thomas-lebeau
force-pushed
the
worktree-squishy-churning-salamander
branch
from
acbdb01 to
b81442b
Compare
2 participants
Motivation
@datadog/js-coreis consumed externally by other Datadog SDKs and has a semver stability guarantee. Without an automated API surface check, accidental breaking changes (removed exports, changed signatures) can slip through undetected.Changes
@microsoft/api-extractoras a dev dependencyscripts/check-js-core-api.tsscript that runs API Extractor over each sub-path entry point (cjs/entries/*.d.ts) and compares against committed golden filespackages/js-core/api/for the three existing sub-paths (monitor,time,util)yarn api:checkcommandscheck-js-core-apiCI job in.gitlab-ci.ymlpackages/js-core/AGENTS.mdwith documentation on the new workflowTest instructions
Checklist