Skip to content

chore: Replace internal.js shim with explicit export#6632

Open
davidkna-sap wants to merge 15 commits into
mainfrom
davidkna-sap_internal-ts-shim
Open

chore: Replace internal.js shim with explicit export#6632
davidkna-sap wants to merge 15 commits into
mainfrom
davidkna-sap_internal-ts-shim

Conversation

@davidkna-sap

@davidkna-sap davidkna-sap commented May 29, 2026

Copy link
Copy Markdown
Member
  1. Remove the internal.js shims which can cause issues with swc and move to dedicated export map.
  2. To still support /nested/imports where necessary add conditional exports (INTERNAL_ONLY/export/name) tied to a new workspace condition and enable in project tsconfig and jest
  3. This required adjusting jest mocking
  4. This also required moving to more recent moduleResolutions
  5. As ncc does not support conditions, move to rolldown (configured via test-util package helper)
  6. Configure rolldown to rewrite __filename and __dirname to import.meta.*, removing the need for excluding these from the bundle with nested imports.
  7. Disable prettier on generated .github/actions to avoid slowdown

Closes https://github.com/SAP/ai-sdk-js-backlog/issues/576

@davidkna-sap
davidkna-sap force-pushed the davidkna-sap_internal-ts-shim branch 18 times, most recently from d6cacff to 8a05aa8 Compare June 1, 2026 10:14
@davidkna-sap
davidkna-sap marked this pull request as ready for review June 1, 2026 10:31
@davidkna-sap
davidkna-sap force-pushed the davidkna-sap_internal-ts-shim branch from 8a05aa8 to fcd95aa Compare June 1, 2026 10:40
Comment thread packages/generator/package.json Outdated
Comment on lines +24 to +28
"./INTERNAL_ONLY/test/test-util/create-generator-options": {
"workspace": {
"types": "./test/test-util/create-generator-options.ts",
"default": "./test/test-util/create-generator-options.ts"
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[q] What is INTERNAL_ONLY?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For any exported files that requires the workspace condition (not importable directly by third party packages, afaik even transitively), I added this so it's easier to spot in diffs so it does not end up in any of the non-test production code.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I read up on this. Really nice! What I don't like though, is that you have to import it using "INTERNAL_ONLY". Wouldn't it be nicer to export everything like (aka. directly) this for tests?
I would expect something along the lines of:

 ".": {
      "types": "./dist/index.d.ts",
      "default": "./dist/index.js",
      "workspace": {
        "types": "./internal.ts",
        "default": "./internal.ts"
      }
    },
  "./test/test-util/create-generator-options": {
      "workspace": {
        "types": "./test/test-util/create-generator-options.ts",
        "default": "./test/test-util/create-generator-options.ts"
      },

I am not even sure you can configure it this way and it would be ok to consider something like this a follow up.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't need to have that prefix, but I wanted to make it very visible as tests will also have the condition enabled; and I wanted to ensure that it's not ever imported in production code.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For me personally it is clear that test/test-util should not be imported in production code. Even clearer than INTERNAL_ONLY, because internal does not tell me that it is only for tests. Due to the workspace condition it wouldn't affect users only us, right?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've now removed INTERNAL_ONLY

…-ts-shim

* origin/main:
  chore(deps): bump js-yaml from 4.1.1 to 4.2.0 (#6647)
  chore(deps): bump eslint-plugin-jsdoc from 62.9.0 to 63.0.0 (#6628)
  chore(deps): bump bignumber.js from 11.1.1 to 11.1.2 (#6644)
  chore: Set explicit workflow permissions (#6629)
  chore(deps-dev): bump turbo from 2.9.15 to 2.9.16 (#6645)
  chore(deps): bump memfs from 4.57.2 to 4.57.3 (#6643)
  chore(deps): bump eslint-plugin-prettier from 5.5.5 to 5.5.6 (#6641)
  chore(deps-dev): bump eslint from 10.4.0 to 10.4.1 (#6642)
  chore(deps-dev): bump @sap/cds-dk from 9.9.1 to 9.9.2 (#6640)
This reverts commit 78d508c.
@davidkna-sap
davidkna-sap force-pushed the davidkna-sap_internal-ts-shim branch from 8b88100 to 10dc040 Compare June 5, 2026 12:41
…-ts-shim

* origin/main:
  chore: Remove node 20 from build matrix (#6662)
  chore(deps-dev): bump @types/node from 22.19.19 to 22.19.20 (#6660)
  chore(deps): bump bignumber.js from 11.1.2 to 11.1.3 (#6659)
  chore(deps): bump github/codeql-action from 4.36.1 to 4.36.2 (#6658)
  chore(deps): bump rtCamp/action-slack-notify from 2.3.3 to 2.4.0 (#6657)
  Update dependabot.yml
  chore(deps): bump semver from 7.8.1 to 7.8.2 (#6656)
  chore(deps): bump actions/checkout from 6.0.2 to 6.0.3 (#6649)
  chore(deps): bump @typescript-eslint/parser from 8.60.0 to 8.60.1 (#6654)
  chore(deps): bump typescript-eslint from 8.60.0 to 8.60.1 (#6651)
  chore(deps): bump memfs from 4.57.3 to 4.57.6 (#6655)
  chore(deps): bump axios from 1.16.1 to 1.17.0 (#6653)
  chore(deps): bump eslint-plugin-jsdoc from 63.0.0 to 63.0.1 (#6652)
  chore(deps): bump eslint-import-resolver-typescript from 4.4.4 to 4.4.5 (#6650)
  chore(deps): bump github/codeql-action from 4.36.0 to 4.36.1 (#6648)
davidkna-sap and others added 6 commits June 17, 2026 16:52
…-ts-shim

* origin/main:
  chore: Discover git-hashes in multi-line release notes (#6621)
  chore(deps-dev): bump eslint from 10.4.1 to 10.5.0 (#6680)
  chore(deps): bump memfs from 4.57.6 to 4.57.7 (#6678)
  chore(deps-dev): bump turbo from 2.9.16 to 2.9.18 (#6677)
  chore: Improve performance of `check-pr` (#6664)
  chore: change long workflow name (#6676)
  chore(deps): bump prettier from 3.8.3 to 3.8.4 (#6672)
  chore(deps): bump @typescript-eslint/parser from 8.60.1 to 8.61.0 (#6671)
  chore: Fix pnpm lockfile (#6675)
  chore(deps-dev): bump @types/node from 22.19.20 to 22.19.21 (#6669)
  chore(deps): bump semver from 7.8.2 to 7.8.4 (#6667)
  chore(deps): bump typescript-eslint from 8.60.1 to 8.61.0 (#6668)
  chore(deps): bump eslint-plugin-jsdoc from 63.0.1 to 63.0.2 (#6665)
  chore: delete blackduck.yml (#6661)
…-ts-shim

* origin/main:
  chore(deps): bump fast-xml-parser from 5.9.2 to 5.9.3 (#6700)
  chore(deps): bump semver from 7.8.4 to 7.8.5 (#6696)
  chore(deps): bump yargs from 17.7.2 to 17.7.3 (#6697)
  chore(deps): bump @typescript-eslint/parser from 8.61.0 to 8.61.1 (#6692)
  chore(deps): bump eslint-plugin-jsdoc from 63.0.2 to 63.0.6 (#6693)
  chore(deps): bump axios from 1.17.0 to 1.18.0 (#6691)
  chore(deps): bump typescript-eslint from 8.61.0 to 8.61.1 (#6690)
  chore(deps): bump bignumber.js from 11.1.3 to 11.1.4 (#6689)
  chore(deps): bump fast-xml-parser from 5.8.0 to 5.9.2 (#6688)
  chore(deps-dev): bump http-proxy-middleware from 3.0.5 to 3.0.7 (#6686)
  chore: Resolve audit warnings (#6683)
  chore(setup): Assume availability of registry mirror in more cases (#6682)
  chore(deps-dev): bump multer from 2.1.1 to 2.2.0 (#6685)
  chore: Remove `puppeteer` and fix `npm-stats` script (#6646)
Resolve conflicts: remove @vercel/ncc in favour of rolldown/test-util-build-internal,
keep deleted index.js shims, and restore root tsconfig.json.
…-ts-shim

* origin/main:
  feat(deps): bump eslint-plugin-jsdoc from 63.0.10 to 63.0.13 (#6751)
  feat(deps-dev): bump typedoc from 0.28.19 to 0.28.20 (#6752)
  feat(deps-dev): bump @types/node from 22.20.0 to 22.20.1 (#6756)
  feat(deps): bump the codeql group across 1 directory with 3 updates (#6763)
  chore: Remove devEngines constraint again (#6760)
  chore: Only allow single pnpm version for dependabot (corepack) (#6761)
  chore: re-add pnpm-version option (#6762)
  chore: group codeql action updates (#6759)
  feat(deps): bump typescript to v6/v7 (#6472)
  chore: Try to fix dependabot pnpm handling (#6747)
  chore(deps): bump actions/setup-node in /.github/actions/setup (#6754)
  chore(deps): bump bignumber.js from 11.1.4 to 11.1.5 (#6749)
  chore(deps): bump actions/checkout from 6.0.3 to 7.0.0 (#6708)
  chore(deps-dev): bump @cap-js/sqlite from 2.4.0 to 3.0.2 (#6739)
  chore(deps-dev): bump @sap/cds-dk from 9.9.2 to 10.0.3 (#6736)
Comment thread packages/generator/package.json Outdated
Comment on lines +24 to +28
"./INTERNAL_ONLY/test/test-util/create-generator-options": {
"workspace": {
"types": "./test/test-util/create-generator-options.ts",
"default": "./test/test-util/create-generator-options.ts"
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For me personally it is clear that test/test-util should not be imported in production code. Even clearer than INTERNAL_ONLY, because internal does not tell me that it is only for tests. Due to the workspace condition it wouldn't affect users only us, right?

import { join, resolve } from 'path';
import { createOptions } from '@sap-cloud-sdk/generator/test/test-util/create-generator-options';
import { generate } from '@sap-cloud-sdk/generator/src';
import { createOptions } from '../../packages/generator/test/test-util/create-generator-options';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[q] Why is this no longer referencing the generator? Or INTERNAL_ONLY?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this was either to avoid circular dependencies or to allow mocking.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

…-ts-shim

* origin/main:
  fix(openapi): Return `Blob` instead of `Buffer` (matching external types) (#6663)
  chore(deps): bump fast-xml-parser from 5.9.3 to 5.10.0 (#6780)
  chore: merge dependabot dev and prod config (#6779)
  chore: fix dependabot config (#6778)
  chore: add dev dependency config (#6777)
  chore(deps-dev): bump turbo from 2.10.0 to 2.10.4 (#6758)
  chore(deps): bump typescript-eslint from 8.62.1 to 8.63.0 (#6774)
  chore(deps-dev): bump @sap/cds-dk from 10.0.3 to 10.0.4 (#6772)
  chore(deps): bump memfs from 4.57.8 to 4.64.0 (#6769)
  chore(deps): bump @typescript-eslint/parser from 8.62.1 to 8.63.0 (#6776)
  chore(deps-dev): bump @eslint/eslintrc from 3.3.5 to 3.3.6 (#6773)
  chore(deps): bump prettier from 3.9.4 to 3.9.5 (#6771)
  chore(deps-dev): bump eslint from 10.6.0 to 10.7.0 (#6768)
  chore(deps): bump content-type and @types/content-type (#6608)
@davidkna-sap
davidkna-sap requested a review from marikaner July 17, 2026 06:30
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

Successfully merging this pull request may close these issues.

3 participants