Skip to content

start emitting declarations during the build - extracted from the JS files #417

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
NullVoxPopuli merged 1 commit into from
Mar 25, 2026
+2,551 −384
Merged

start emitting declarations during the build - extracted from the JS files #417

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 14 additions & 5 deletions ember-moment/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,15 @@
"license": "MIT",
"author": "",
"exports": {
".": "./dist/index.js",
"./*": "./dist/*.js",
"./addon-main.js": "./addon-main.cjs"
".": {
"types": "./declarations/index.d.ts",
"default": "./dist/index.js"
},
"./addon-main.js": "./addon-main.cjs",
"./*": {
"types": "./declarations/*.d.ts",
"default": "./dist/*.js"
}
Comment on lines +18 to +22
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 24, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's locate and read the package.json file
find . -name "package.json" -type f | head -20

Repository: adopted-ember-addons/ember-moment

Length of output: 203

🏁 Script executed:

# Read the main package.json to inspect the export map
cat -n ./package.json | head -100

Repository: adopted-ember-addons/ember-moment

Length of output: 1435

🏁 Script executed:

# Read the ember-moment/package.json file which is the one being reviewed
cat -n ./ember-moment/package.json

Repository: adopted-ember-addons/ember-moment

Length of output: 4987

🏁 Script executed:

# Check the tsconfig.json to understand declaration generation
cat -n ./ember-moment/tsconfig.json

Repository: adopted-ember-addons/ember-moment

Length of output: 897

🏁 Script executed:

# Check the build output and directory structure
ls -la ./ember-moment/dist/ 2>/dev/null || echo "dist/ doesn't exist yet"
ls -la ./ember-moment/declarations/ 2>/dev/null || echo "declarations/ doesn't exist yet"

Repository: adopted-ember-addons/ember-moment

Length of output: 132

🏁 Script executed:

# Check the rollup configuration to understand the build output structure
cat -n ./ember-moment/rollup.config.mjs | head -80

Repository: adopted-ember-addons/ember-moment

Length of output: 3043

🏁 Script executed:

# Let's check if there's a build output or if we can inspect the src directory structure
ls -la ./ember-moment/src/ | head -30

Repository: adopted-ember-addons/ember-moment

Length of output: 415

🏁 Script executed:

# Let's check what files are in the src/helpers and src/services directories
find ./ember-moment/src -type f -name "*.js" -o -name "*.gjs" | head -20

Repository: adopted-ember-addons/ember-moment

Length of output: 918

🌐 Web query:

TypeScript package.json exports longest prefix pattern matching module resolution

💡 Result:

TypeScript's module resolution for package.json "exports" field uses longest prefix pattern matching, analogous to its "paths" mapping resolution. In TypeScript's module resolution (under moduleResolution: "node16", "nodenext", or "bundler"), when resolving an import specifier against a package's "exports" map, it selects the export key with the longest matching prefix for the subpath. This matches Node.js's PACKAGE_EXPORTS_RESOLVE algorithm, which iterates over exports keys (in package.json object order) and uses the first (longest-matching) subpath pattern or exact match. For subpath patterns (keys ending in ""), matching replaces "" with the corresponding suffix from the requested subpath. TypeScript prioritizes "exports" over legacy fields like "typesVersions", "types", and "main" when present. To enable: Use "moduleResolution": "node16"/"nodenext"/"bundler" and "resolvePackageJsonExports": true (default in those modes). Example package.json: { "exports": { ".": "./index.js", "./foo": "./foo.js", "./bar/": "./bar/.js" } } Import "pkg/bar/baz" matches "./bar/*" (longest prefix "./bar/" > "."). This prevents access to unexported subpaths, enforcing encapsulation.

Citations:

Add explicit ./_app_/* export to prevent false type advertisements.

The wildcard ./* export pattern matches _app_/* paths (from ember-addon.app-js), but TypeScript's longest-prefix matching means if ./_app_/* is defined, it takes precedence. Currently, the ./* pattern advertises declarations/_app_/**.d.ts types that won't be generated—the build only creates declarations from src/ (via tsconfig.json's rootDir: "./src"), not from the _app_/ build artifact. Add an explicit JS-only ./_app_/* export before ./* to carve those paths out cleanly and prevent false type resolution.

📦 Suggested change 
   "exports": {
     ".": {
       "types": "./declarations/index.d.ts",
       "default": "./dist/index.js"
     },
     "./addon-main.js": "./addon-main.cjs",
+    "./_app_/*": "./dist/_app_/*.js",
     "./*": {
       "types": "./declarations/*.d.ts",
       "default": "./dist/*.js"
     }
   },
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"./addon-main.js": "./addon-main.cjs",
"./*": {
"types": "./declarations/*.d.ts",
"default": "./dist/*.js"
}
"./addon-main.js": "./addon-main.cjs",
"./_app_/*": "./dist/_app_/*.js",
"./*": {
"types": "./declarations/*.d.ts",
"default": "./dist/*.js"
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@ember-moment/package.json` around lines 18 - 22, Add an explicit JS-only
export entry for the "./_app_/*" paths before the existing "./*" export in the
package.json "exports" block so TypeScript won't match "./_app_/*" into the
wildcard and incorrectly advertise declarations; specifically, carve out
"./_app_/*" with only a runtime/default export (no "types" entry) and place it
above the "./*" mapping that currently provides "types": "./declarations/*.d.ts"
and "default": "./dist/*.js" to ensure declarations from "declarations/" are not
(falsely) applied to _app_ artifacts.

},
"files": [
"addon-main.cjs",
Expand All @@ -39,10 +45,12 @@
"@babel/core": "^7.24.7",
"@babel/eslint-parser": "^7.24.7",
"@babel/runtime": "^7.24.7",
"@embroider/addon-dev": "^4.3.1",
"@ember/library-tsconfig": "^2.0.0",
"@embroider/addon-dev": "^8.3.0",
"@rollup/plugin-babel": "^6.0.4",
"babel-plugin-ember-template-compilation": "^2.2.5",
"concurrently": "^8.2.2",
"ember-source": "^6.11.0",
"ember-template-lint": "^6.0.0",
"eslint": "^8.56.0",
"eslint-config-prettier": "^9.1.0",
Expand All @@ -53,7 +61,8 @@
"prettier": "^3.3.2",
"prettier-plugin-ember-template-tag": "^2.0.2",
"rollup": "^4.18.0",
"rollup-plugin-copy": "^3.5.0"
"rollup-plugin-copy": "^3.5.0",
"typescript": "^6.0.2"
},
"peerDependencies": {
"moment": "^2",
Expand Down
5 changes: 2 additions & 3 deletions ember-moment/rollup.config.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,8 @@ export default {
// Ensure that .gjs files are properly integrated as Javascript
addon.gjs(),

// addons are allowed to contain imports of .css files, which we want rollup
// to leave alone and keep in the published output.
addon.keepAssets(['**/*.css']),
// Emit .d.ts declaration files
addon.declarations('declarations', 'tsc --declaration'),
Comment on lines +56 to +57
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 24, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# Check if the file exists and examine it
find . -name "rollup.config.mjs" -type f

Repository: adopted-ember-addons/ember-moment

Length of output: 109

🏁 Script executed:

# Read the rollup.config.mjs file to understand context
cat -n ember-moment/rollup.config.mjs 2>/dev/null || find . -name "rollup.config.mjs" -exec cat -n {} +

Repository: adopted-ember-addons/ember-moment

Length of output: 3043

🏁 Script executed:

# Look for package.json to understand dependencies
find . -name "package.json" -type f | head -5

Repository: adopted-ember-addons/ember-moment

Length of output: 203

🏁 Script executed:

# Check for tsconfig files
find . -name "tsconfig.json" -type f

Repository: adopted-ember-addons/ember-moment

Length of output: 105

🏁 Script executed:

# Read the main tsconfig.json
cat -n tsconfig.json 2>/dev/null || cat -n ./ember-moment/tsconfig.json 2>/dev/null || find . -name "tsconfig.json" -exec cat -n {} +

Repository: adopted-ember-addons/ember-moment

Length of output: 897

🌐 Web query:

TypeScript emitDeclarationOnly --declaration allowJs documentation

💡 Result:

The TypeScript compiler options --emitDeclarationOnly, --declaration, and allowJs work together to generate declaration files (.d.ts) from JavaScript files without emitting JavaScript output. Key points: - emitDeclarationOnly: true (or --emitDeclarationOnly): Instructs the compiler to only emit .d.ts files, skipping .js files. Useful when using another transpiler for JS or providing types for consumers. - declaration: true (or --declaration): Enables generation of .d.ts files. Required when using emitDeclarationOnly. - allowJs: true (or --allowJs): Allows the compiler to process .js files as part of the program, enabling type checking and declaration generation from them. To generate .d.ts files from .js files: 1. Create a tsconfig.json: { "compilerOptions": { "allowJs": true, "declaration": true, "emitDeclarationOnly": true, "outDir": "dist" // Optional: directs output to a separate directory }, "include": ["src//.js"] } 2. Run tsc (uses tsconfig.json) or CLI: tsc src//.js --allowJs --declaration --emitDeclarationOnly --outDir dist. This setup processes .js files (thanks to allowJs), generates corresponding .d.ts files (via declaration + emitDeclarationOnly), and avoids overwriting original .js files (use outDir). Examples from docs: - CLI: tsc index.js --declaration --emitDeclarationOnly - For JS projects incrementally adopting TS. Note: Cannot combine emitDeclarationOnly with noEmit. For jsconfig.json projects, there may be compatibility issues with --emitDeclarationOnly. Official docs confirm this combination for creating .d.ts from .js files, especially with JSDoc for better types.

Citations:

Add --emitDeclarationOnly to the declaration generation command.

This build uses Rollup/Babel to emit JavaScript. When TypeScript generates declarations from JavaScript with allowJs enabled, it should run in declaration-only mode to avoid emitting JavaScript that would conflict with Rollup's output. TypeScript's documentation explicitly recommends pairing --declaration with --emitDeclarationOnly in this scenario.

Suggested change 
-    addon.declarations('declarations', 'tsc --declaration'),
+    addon.declarations('declarations', 'tsc --declaration --emitDeclarationOnly'),
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Emit .d.ts declaration files
addon.declarations('declarations', 'tsc --declaration'),
// Emit .d.ts declaration files
addon.declarations('declarations', 'tsc --declaration --emitDeclarationOnly'),
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@ember-moment/rollup.config.mjs` around lines 56 - 57, Update the TypeScript
declaration-generation command used by addon.declarations to include
--emitDeclarationOnly so tsc only emits .d.ts files and does not produce JS that
conflicts with Rollup/Babel; locate the call addon.declarations('declarations',
'tsc --declaration') and change the command string to include
--emitDeclarationOnly (e.g., 'tsc --declaration --emitDeclarationOnly').


// Remove leftover build artifacts when starting a new build.
addon.clean(),
Expand Down
22 changes: 22 additions & 0 deletions ember-moment/tsconfig.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
{
"extends": "@ember/library-tsconfig",
"include": ["./src/**/*", "./unpublished-development-types/**/*"],
"compilerOptions": {
"allowJs": true,
"declarationDir": "declarations",

/**
https://www.typescriptlang.org/tsconfig#rootDir
"Default: The longest common path of all non-declaration input files."

Because we want our declarations' structure to match our rollup output,
we need this "rootDir" to match the "srcDir" in the rollup.config.mjs.

This way, we can have simpler `package.json#exports` that matches
imports to files on disk
*/
"rootDir": "./src",

"types": ["ember-source/types"]
}
}
Loading
Loading