Skip to content

Convert ambient module declarations to proper TypeScript module files#3189

Open
robigan wants to merge 9 commits into
salesforce:masterfrom
robigan:master
Open

Convert ambient module declarations to proper TypeScript module files#3189
robigan wants to merge 9 commits into
salesforce:masterfrom
robigan:master

Conversation

@robigan
Copy link
Copy Markdown

@robigan robigan commented May 20, 2026

This properly fixes TypeScript declarations for components imported with the quick setup usage pattern as demonstrated in the README

### Quick Setup (ES6 and CJS modules)
For a no hassle setup and compatibility with Create React App, transpiled ES6 and CommonJS module versions have been included within the NPM package. If using this setup, please re-write the `import` statement in the documentation site examples. Use the following named `import` syntax to access the transpiled components from `/lib/index.js`:
```
import { Button } from '@salesforce/design-system-react';
<Button label="Hello Button" />
```

The problem lies in the fact that ambient module declarations are exclusively matched against the import specifier exactly. import { Button } from '@salesforce/design-system-react'; won't match against declare module '@salesforce/design-system-react/components/button' because the import specifiers do not match exactly. import Button from '@salesforce/design-system-react/components/button'; would instead work.

There is also a problem for ESM module users whereby the declaration files in the /module directory do not contain these prefixes and fail to resolve the matching TS declaration; by making the ambient modules proper ESM/TS modules, the imports become relative and no longer rely on matching the absolute path. Thus, properly fixing #3113.

Motivation

It's true that one could continue with the advanced usage pattern described in the README, but I would argue this is anti DX.

One, a package should be a black box when developing software, I'd say that a developer shouldn't have to know the internal structure of a package for them to use that package and reap the DX benefits technology stacks like TypeScript offer. I should just be able to import the stuff I need from the import specifier of the module and be on my merry way.

Two, in my personal experience, weird bugs arise, and some systems like Yarn PnP get pissy, when importing internal structures from a module.

Three, it seems that although there is little activity in this repository in the last few months, this repo can still be a platform for the Salesforce team to push SLDS 2 React components to, by pushing for users of this library to use the quick setup usage pattern, it gives maintainers flexibility to change repository structure how they see fit without breaking usage of this library.

Four, I'd argue that the "quick setup" usage pattern is the TypeScript best practice, when writing code that uses this library, tools like Visual Studio Code autoimport missing components using the module import specifier. Having to manually change to the advanced usage pattern can be quite cumbersome.

Thanks for listening to my Ted talk.


Large Language Model declaration & Additional Description

This PR was mostly created using LLMs to automate the application of this proposed fix to 183 qualifying ambient module declarations.

NB: This PR is marked as Draft as I've identified an issue that TS complains about that I plan to fix. The PR is being opened for public review whilst I get it ready. Issues have been fixed and tested for.

How I plan to prevent AI slop

I have currently checked about 10% of the changed files and validated that GitHub Copilot has properly applied the idea/modifications I had in mind. I am testing my fork on an enterprise project I am working for based on SF Marketing Cloud Engagement to properly validate that the new TS declarations work in an enterprise project using this library. I plan to manually verify all 183 changed files once I deem this PR ready for review.

How I plan to be transparent about LLM usage

The PR on my fork can be seen at robigan#1 whereby Copilot's own internal review process can be found. The Copilot session can also be found in the PR by clicking "View Session" or by following this link (I am unsure if GH allows others to see the session, but I have linked anyways just in case). Within the Copilot session can be found the prompt I used to make the changes. I am also embedding the prompt just below for transparency.

Edit: The rest of my changes can be seen in robigan#2 and robigan#3

Prompt Used

I will provide you a handoff file describing the work that has been done up to now to identify the component types resolution problem. I want you to convert all TypeScript declaration modules which are ambient modules to proper module declarations in components. Create a new branch and then open a Pull Request into salesforce/design-system-react

Session Summary

Goal: Understand why importing from @salesforce/design-system-react (the main barrel) fails TypeScript resolution, while deep-path imports like @salesforce/design-system-react/components/setup-assistant work correctly.

Accomplished: Root cause identified, fix for maintainers articulated, local workaround confirmed.


Key Decisions Made

No code was changed. This was a pure investigation/advisory session.


Current State

Working:

  • src/App.tsx compiles without errors using the deep-path import workaround on line 8:
    import SetupAssistant from "@salesforce/design-system-react/components/setup-assistant";
  • SetupAssistantStep and IconSettings are imported from the main barrel and appear to work (their .d.ts files may have a different structure — not verified, but no errors reported).

Broken (upstream package bug):

  • import { SetupAssistant } from "@salesforce/design-system-react" does not resolve correct types. The barrel lib/components/index.d.ts re-exports via export { default as SetupAssistant } from './setup-assistant', but lib/components/setup-assistant.d.ts is an ambient module declaration (declare module '@salesforce/design-system-react/components/setup-assistant' { ... }), not a real module file — so the relative re-export finds no top-level exports.

Known Issues & Pitfalls

  • The ambient declare module pattern only satisfies TypeScript when the import specifier exactly matches the string in the declaration. Relative imports from a barrel can't consume ambient declarations as re-export sources.
  • SetupAssistantStep works from the barrel (lib/components/setup-assistant/step.d.ts was not inspected — it may be structured differently or TypeScript is silently resolving it as any). Worth verifying.
  • The package has no types, typings, or exports field in package.json — only main and module. This means TypeScript falls back to co-located .d.ts discovery, which compounds the resolution problem under moduleResolution: bundler.

Lessons Learned

  • declare module 'specifier' { ... } in a .d.ts file is an ambient module declaration — it only activates when the import specifier matches exactly. It does not make the .d.ts file itself a module with exports.
  • A .d.ts file becomes a proper module only when it has top-level import or export statements. Without them, it's a "script" file, and declare module blocks inside it are ambient globals.
  • The @salesforce/design-system-react package structure has a historical design decision: types were hand-written as ambient declarations keyed to the components/* deep paths (matching the source layout), rather than generated to match the compiled lib/components/* output layout.

Next Steps

  1. Open an issue on github.com/salesforce/design-system-react describing:

    • The barrel lib/components/index.d.ts re-exports via relative paths, but the co-located .d.ts files use declare module ambient syntax, making them incompatible as re-export sources.
    • Suggested fix: convert all lib/components/*.d.ts files from ambient declarations to proper module .d.ts files (remove the declare module wrapper, hoist contents to top level).
    • Optional addition: add exports + typesVersions to package.json to formally map both the main entry and components/* deep paths.
  2. Verify SetupAssistantStep from the barrel actually has correct types (not silently any) by checking lib/components/setup-assistant/step.d.ts.

  3. Local workaround: Continue using deep-path imports for any component where the barrel import loses types. Optionally consolidate them in a shim file like src/lib/slds.ts to keep component files clean.


Context for Next Session

  • Repo: customactivity-web
  • Key file: src/App.tsx — line 8 is the workaround deep-path import
  • Package under investigation: @salesforce/design-system-react@0.10.65
  • Relevant package files:
    • index.d.ts — barrel, uses relative re-exports
    • setup-assistant.d.ts — ambient declaration (the broken file)
    • setup-assistant.d.ts — same ambient declaration, different path
  • tsconfig: moduleResolution: bundler, skipLibCheck: true (errors are suppressed at the lib level — actual resolution failure would surface as any or missing props, not a hard compile error)
  • No code was modified this session. The workspace is clean.

CONTRIBUTOR checklist (do not remove)

Please complete for every pull request

  • First-time contributors should sign the Contributor License Agreement. It's a fancy way of saying that you are giving away your contribution to this project. If you haven't before, wait a few minutes and a bot will comment on this pull request with instructions.
  • npm run lint:fix has been run and linting passes.
  • Mocha, Jest (Storyshots), and components/component-docs.json CI tests pass (npm test). (NB: See comments)
  • Tests have been added for new props to prevent regressions in the future. See readme. No new props have been added
  • Review the appropriate Storybook stories. Open http://localhost:9001/.
  • Review tests are passing in the browser. Open http://localhost:8001/.
  • Review markup conforms to SLDS by looking at DOM snapshot strings.

REVIEWER checklist (do not remove)

  • CircleCI tests pass. This includes linting, Mocha, Jest, Storyshots, and components/component-docs.json tests.
  • Tests have been added for new props to prevent regressions in the future. See readme.
  • Review the appropriate Storybook stories. Open http://localhost:9001/.
  • The Accessibility panel of each Storybook story has 0 violations (aXe). Open http://localhost:9001/.
  • Review tests are passing in the browser. Open http://localhost:8001/.
  • Review markup conforms to SLDS by looking at DOM snapshot strings.
Required only if there are markup / UX changes
  • Add year-first date and commit SHA to last-slds-markup-review in package.json and push.
  • Request a review of the deployed Heroku app by the Salesforce UX Accessibility Team.
  • Add year-first review date, and commit SHA, last-accessibility-review, to package.json and push.
  • While the contributor's branch is checked out, run npm run local-update within locally cloned site repo to confirm the site will function correctly at the next release.

Copilot AI and others added 2 commits May 20, 2026 08:19
…-declarations

Convert ambient module declarations to proper TypeScript module files
@salesforce-cla
Copy link
Copy Markdown

Thanks for the contribution! Before we can merge this, we need @robigan to sign the Salesforce Inc. Contributor License Agreement.

Copilot AI and others added 7 commits May 21, 2026 06:24
fix: add `declare` modifier to top-level function declarations in .d.ts files
…espace-error

Co-authored-by: robigan <35210888+robigan@users.noreply.github.com>
Qualify JSX namespace with React prefix in all .d.ts files
@robigan
Copy link
Copy Markdown
Author

robigan commented May 21, 2026

I was unable to do automated tests because Chrome Headless was unable to launch given that I did the tests on a GitHub Codespace without polluting my machine. I ran the test suite in Firefox and Chromium by navigating to localhost:8001 and running the tests there. Is that fine?

image

@robigan robigan marked this pull request as ready for review May 22, 2026 07:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants