feat(angular): Design Angular version of phone mask library#106
Open
stefashkaa wants to merge 24 commits into
Open
feat(angular): Design Angular version of phone mask library#106stefashkaa wants to merge 24 commits into
stefashkaa wants to merge 24 commits into
Conversation
…ve and add COUNTRY_DETECTION
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds a full Angular package (component, directive, pipe, services), demo app, comprehensive tests, and integrates repo tooling/config/docs and test path aliases across packages. ChangesAngular Phone Mask Library
Monorepo Tooling and Cross-Package Tests
Sequence Diagram(s)sequenceDiagram
participant User
participant NgComponent as PhoneInputComponent
participant Services as UseCountry/UseFormatter/Handlers
participant Clipboard
User->>NgComponent: type/select/click
NgComponent->>Services: beforeinput/input/keydown/paste
Services-->>NgComponent: digits/display/validity
User->>NgComponent: click copy
NgComponent->>Clipboard: writeText
Clipboard-->>NgComponent: success/failure
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
stefashkaa
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
Adds first-class Angular support to the phone-mask monorepo by introducing a new @desource/phone-mask-angular package that mirrors the existing framework adapters (React/Vue/Svelte) while using Angular-native primitives (standalone component/directive/pipe, signals/model/outputs, DI services, and ControlValueAccessor), plus the associated build/test/demo/tooling integration.
Changes:
- Introduces
@desource/phone-mask-angularwithPhoneInputComponent,PhoneMaskDirective,PhoneMaskPipe, andUsePhoneMaskService, plus internal services and a demo app. - Updates workspace/config/tooling to recognize Angular (root tsconfig refs, docs, ESLint config, pnpm hoisting tweaks, gitignore).
- Normalizes unit/e2e tests to use
@src/@commonaliases (and adds missing aliases where needed, e.g. corephone-maskvitest config + tests tsconfig paths).
Reviewed changes
Copilot reviewed 84 out of 87 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.json | Adds TS project reference for the new Angular package. |
| README.md | Lists Angular as supported and documents Angular install/usage. |
| pnpm-workspace.yaml | Expands onlyBuiltDependencies to cover additional native deps. |
| packages/phone-mask/vitest.config.ts | Adds @common/@src aliases for core package tests. |
| packages/phone-mask/tests/unit/utils.test.ts | Switches test imports to @src/* aliases. |
| packages/phone-mask/tests/unit/handlers.test.ts | Switches test imports to @src/* aliases. |
| packages/phone-mask/tests/unit/geoip.test.ts | Switches test imports to @src/* aliases. |
| packages/phone-mask/tests/unit/formatter.test.ts | Switches test imports to @src/* aliases. |
| packages/phone-mask/tests/unit/exports.test.ts | Switches test imports to @src/* aliases. |
| packages/phone-mask/tests/unit/entries.test.ts | Switches test imports to @src/* aliases. |
| packages/phone-mask/tests/unit/data-min.test.ts | Switches test imports to @src/* aliases. |
| packages/phone-mask/tests/unit/country-selector.test.ts | Switches test imports to @src/* and @common/* aliases. |
| packages/phone-mask/tests/unit/country-code-emodji.test.ts | Switches test imports to @src/* aliases. |
| packages/phone-mask/tests/tsconfig.json | Adds TS path mappings for @common/* and @src/* in tests. |
| packages/phone-mask-vue/tests/unit/usePhoneMask.test.ts | Switches test import to @src/* alias. |
| packages/phone-mask-vue/tests/unit/PhoneInput.test.ts | Switches test imports to @src/* aliases. |
| packages/phone-mask-vue/tests/unit/index.test.ts | Switches test imports to @src/* aliases. |
| packages/phone-mask-vue/tests/e2e/UsePhoneMask.spec.ts | Switches e2e import to @common/* alias. |
| packages/phone-mask-vue/tests/e2e/PhoneInput.spec.ts | Switches e2e import to @common/* alias. |
| packages/phone-mask-svelte/tests/unit/usePhoneMask.test.ts | Switches test import to @src/* alias. |
| packages/phone-mask-svelte/tests/unit/useCopyAction.test.ts | Switches test import to @src/* alias. |
| packages/phone-mask-svelte/tests/unit/index.test.ts | Switches test imports to @src/* aliases. |
| packages/phone-mask-svelte/tests/e2e/UsePhoneMask.spec.ts | Switches e2e import to @common/* alias. |
| packages/phone-mask-svelte/tests/e2e/PhoneInput.spec.ts | Switches e2e import to @common/* alias. |
| packages/phone-mask-react/tests/unit/usePhoneMask.test.tsx | Switches test import to @src/* alias. |
| packages/phone-mask-react/tests/unit/PhoneInput.test.tsx | Switches test imports to @src/* aliases. |
| packages/phone-mask-react/tests/unit/index.test.ts | Switches test imports to @src/* aliases. |
| packages/phone-mask-react/tests/e2e/UsePhoneMask.spec.ts | Switches e2e import to @common/* alias. |
| packages/phone-mask-react/tests/e2e/PhoneInput.spec.ts | Switches e2e import to @common/* alias. |
| packages/phone-mask-angular/vitest.angular.config.ts | Adds Angular Vitest config with @common/@src aliases and coverage. |
| packages/phone-mask-angular/tsconfig.spec.json | Adds Angular spec tsconfig for unit tests. |
| packages/phone-mask-angular/tsconfig.json | Adds Angular library tsconfig with strict compiler options. |
| packages/phone-mask-angular/tests/unit/useValidationHint.test.ts | Adds unit tests for validation hint service behavior. |
| packages/phone-mask-angular/tests/unit/useTimer.test.ts | Adds unit tests for timer utility service. |
| packages/phone-mask-angular/tests/unit/useTheme.test.ts | Adds unit tests for theme service + default behavior. |
| packages/phone-mask-angular/tests/unit/usePhoneMask.test.ts | Adds unit tests for UsePhoneMaskService API and event wiring. |
| packages/phone-mask-angular/tests/unit/useInputHandlers.test.ts | Adds unit tests for Angular input handler service edge cases. |
| packages/phone-mask-angular/tests/unit/useFormatter.test.ts | Adds unit tests for formatter service behavior and scheduling. |
| packages/phone-mask-angular/tests/unit/useCountrySelector.test.ts | Adds unit tests for country selector service + DOM behavior integration. |
| packages/phone-mask-angular/tests/unit/useCountry.test.ts | Adds unit tests for country selection/detection service. |
| packages/phone-mask-angular/tests/unit/useCopyAction.test.ts | Adds unit tests for copy action service + live region behavior. |
| packages/phone-mask-angular/tests/unit/useClipboard.test.ts | Adds unit tests for clipboard service including fallback behavior. |
| packages/phone-mask-angular/tests/unit/setup/tools.ts | Adds Angular-specific testing tools adapter for shared test suites. |
| packages/phone-mask-angular/tests/unit/setup/angular.ts | Initializes Angular testing environment for Vitest runner. |
| packages/phone-mask-angular/tests/unit/service-pipe.test.ts | Adds unit tests for PhoneMaskPipe formatting modes/inputs. |
| packages/phone-mask-angular/tests/unit/phoneMaskDirective.test.ts | Adds unit tests for directive binding + CVA integration. |
| packages/phone-mask-angular/tests/unit/PhoneInput.test.ts | Adds unit tests for component behavior and CVA integration. |
| packages/phone-mask-angular/tests/unit/index.test.ts | Adds export-surface tests for Angular package entry points. |
| packages/phone-mask-angular/tests/tsconfig.json | Adds Angular test tsconfig with @common/*/@src/* paths. |
| packages/phone-mask-angular/tests/e2e/UsePhoneMask.spec.ts | Adds Playwright e2e test wiring for service demo. |
| packages/phone-mask-angular/tests/e2e/PhoneInput.spec.ts | Adds Playwright e2e test wiring for component playground. |
| packages/phone-mask-angular/src/types.ts | Defines Angular package public types for component/directive/pipe APIs. |
| packages/phone-mask-angular/src/services/utility/useTimer.service.ts | Adds timer utility service with destroy cleanup. |
| packages/phone-mask-angular/src/services/utility/useClipboard.service.ts | Adds clipboard service with navigator + textarea fallback. |
| packages/phone-mask-angular/src/services/usePhoneMask.service.ts | Adds standalone service API that can attach to an input element. |
| packages/phone-mask-angular/src/services/internal/useValidationHint.service.ts | Adds internal validation hint scheduling service. |
| packages/phone-mask-angular/src/services/internal/useTheme.service.ts | Adds internal theme resolution service (system vs explicit). |
| packages/phone-mask-angular/src/services/internal/useInputHandlers.service.ts | Adds Angular wrapper service over kit input processing helpers. |
| packages/phone-mask-angular/src/services/internal/useFormatter.service.ts | Adds internal formatter state service with clamping + derived values. |
| packages/phone-mask-angular/src/services/internal/useCountrySelector.service.ts | Adds internal country dropdown/search behavior service. |
| packages/phone-mask-angular/src/services/internal/useCountry.service.ts | Adds internal country selection + optional detection via DI token. |
| packages/phone-mask-angular/src/services/internal/useCopyAction.service.ts | Adds internal copy action service + live region announcement. |
| packages/phone-mask-angular/src/public-api.ts | Defines Angular package root public exports. |
| packages/phone-mask-angular/src/phone-mask.pipe.ts | Adds standalone pipe for formatting in templates. |
| packages/phone-mask-angular/src/phone-mask.directive.ts | Adds standalone directive (incl. CVA) for masking existing inputs. |
| packages/phone-mask-angular/src/core.ts | Adds secondary entry-like re-export surface (core + kit). |
| packages/phone-mask-angular/src/components/phone-input/phone-input.component.ts | Adds standalone phone input component implementation (signals, CVA, dropdown, copy/clear). |
| packages/phone-mask-angular/src/components/phone-input/phone-input.component.scss | Adds default component styles (global due to no encapsulation). |
| packages/phone-mask-angular/src/components/phone-input/phone-input.component.html | Adds component template (input + country selector + dropdown). |
| packages/phone-mask-angular/README.md | Adds Angular package documentation and usage examples. |
| packages/phone-mask-angular/playwright.config.ts | Adds Playwright config for Angular demo e2e testing. |
| packages/phone-mask-angular/package.json | Adds package metadata/scripts/peers for Angular adapter. |
| packages/phone-mask-angular/ng-package.json | Adds ng-packagr configuration for library build output. |
| packages/phone-mask-angular/demo/tsconfig.json | Adds demo app tsconfig. |
| packages/phone-mask-angular/demo/src/main.ts | Adds demo bootstrap entrypoint. |
| packages/phone-mask-angular/demo/src/index.html | Adds demo HTML shell. |
| packages/phone-mask-angular/demo/src/app.component.ts | Adds demo playground + service example used by e2e tests. |
| packages/phone-mask-angular/core/src/public-api.ts | Adds core secondary entrypoint exports. |
| packages/phone-mask-angular/core/ng-package.json | Adds ng-packagr secondary entrypoint config. |
| packages/phone-mask-angular/CHANGELOG.md | Adds initial changelog for Angular package. |
| packages/phone-mask-angular/angular.json | Adds Angular CLI project config for lib + demo + vitest runner setup. |
| package.json | Adds angular-eslint to root dev dependencies. |
| eslint.config.js | Adds Angular TS/template linting and inline-template processing. |
| common/tests/unit/setup/tools.ts | Expands MaybeRef to support getter functions (Angular signals). |
| .npmrc | Adds pnpm public hoist rule for tslib to support Angular builds. |
| .gitignore | Ignores Angular CLI cache and demo build output folders. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| --pi-actions-size: 24px; | ||
| } | ||
|
|
||
| .size-compact { |
Comment on lines
+443
to
+449
| // Reduced motion | ||
| @media (prefers-reduced-motion: reduce) { | ||
| * { | ||
| animation-duration: 0.01ms !important; | ||
| animation-iteration-count: 1 !important; | ||
| transition-duration: 0.01ms !important; | ||
| } |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/phone-mask-angular/demo/src/app.component.ts`:
- Around line 440-450: Remove the unnecessary manual change detection: in the
setReadonly method, delete the call to this.changeDetector.detectChanges(); for
setDisabled, try removing the final this.changeDetector.detectChanges() after
updating the signal and calling
this.playgroundPhone()?.setDisabledState(checked) (keep it only if you find a
regression in rendering), then run the demo/tests to verify UI updates; the
relevant symbols are the setDisabled and setReadonly methods,
playgroundPhone()?.setDisabledState and this.changeDetector.detectChanges.
In `@packages/phone-mask-angular/demo/src/main.ts`:
- Line 4: bootstrapApplication(AppComponent) is not handling promise rejections;
update the call to catch and surface bootstrap failures by appending a rejection
handler that logs the error (and exits if desired). Specifically add a .catch
handler on the bootstrapApplication(AppComponent) promise to call console.error
(or your logger) with a clear message and the error object so
provider/configuration errors are visible at startup; reference
bootstrapApplication and AppComponent to locate the call to update.
In `@packages/phone-mask-angular/ng-package.json`:
- Line 2: Update the "$schema" value in ng-package.json to point to the repo
root node_modules for IDE validation: replace the current
"./node_modules/ng-packagr/ng-package.schema.json" reference with
"../../node_modules/ng-packagr/ng-package.schema.json" so IDEs can resolve the
schema for the packages/phone-mask-angular project.
In
`@packages/phone-mask-angular/src/components/phone-input/phone-input.component.ts`:
- Around line 65-74: The component is using input aliasing (countryInput,
localeInput, disabledInput, readOnlyInput etc.) to avoid name conflicts with
exposed computed properties; rename the exposed computed properties instead and
restore clean input names: rename the computed property country →
selectedCountry (and any references to this.country) and locale → currentLocale
(and references to this.locale), then rename the input fields back to their
public names (e.g., countryInput → country, localeInput → locale) and remove the
alias option from their input(...) calls (also remove alias for
disabled/readOnly if present), and update any template/consumer references to
the new computed names so aliasing is no longer required.
- Around line 87-90: The component defines outputs with aliases that collide
with native DOM events—specifically the outputs declared as readonly focused =
output<FocusEvent>({ alias: 'focus' }), readonly blurred = output<FocusEvent>({
alias: 'blur' }), and readonly copiedValue = output<string>({ alias: 'copy'
})—so change these to non-colliding names by removing the alias or using unique
aliases (e.g., remove alias so the outputs are exposed as focused, blurred,
copied or use phoneFocus/phoneBlur/phoneCopy), and update any templates that
currently bind to (focus)/(blur)/(copy) to instead bind to
(focused)/(blurred)/(copied) or the chosen unique names; do the same check for
cleared to ensure its alias (if any) doesn't collide.
In `@packages/phone-mask-angular/src/phone-mask.directive.ts`:
- Around line 197-223: The public methods clear, selectCountry, getDigits,
getFullNumber, getFullFormattedNumber, isComplete, and isValid must guard
against the non-input host case like writeValue/setDisabledState do; update each
to check this.isInput at the start and if false return safe defaults (e.g.,
clear → no-op, selectCountry → false, getters → '' and validity checks → false)
and optionally log a concise console.warn; ensure methods that call setValue or
formatter()/countryState() (e.g., clear → setValue, isComplete →
formatter().isComplete) do not access uninitialized services when this.isInput
is false.
In `@packages/phone-mask-angular/src/phone-mask.pipe.ts`:
- Around line 14-16: The isFormatOptions type guard currently returns true for
arrays and other non-plain objects (like Date), so tighten it by ensuring the
value is a non-null object that is not an Array and has the plain Object
prototype (e.g., !Array.isArray(value) and Object.getPrototypeOf(value) ===
Object.prototype or value.constructor === Object) before asserting
PhoneMaskFormatOptions; update the isFormatOptions function to include these
checks so shorthand country strings still resolve correctly and non-plain
objects don’t bypass the branch that expects a proper options object.
In `@packages/phone-mask-angular/src/services/internal/useFormatter.service.ts`:
- Line 57: The immediate call to emitClampedValue() inside configure
synchronously invokes onChange too early; defer it to match the async pattern
used by the effect by wrapping the call in queueMicrotask(() =>
this.emitClampedValue()) (or otherwise schedule it asynchronously) so that the
initial normalization happens after caller initialization; update the call site
in configure to use queueMicrotask and keep the emitClampedValue, onChange, and
effect logic unchanged.
In
`@packages/phone-mask-angular/src/services/internal/useInputHandlers.service.ts`:
- Around line 86-91: scheduleCaretUpdate uses an untracked setTimeout which can
fire after the service/component is destroyed; update scheduleCaretUpdate to use
a cancellable timer: either store the timer ID returned by setTimeout (in a
property like this.caretTimerId) and clear it (clearTimeout) when the service is
torn down, or replace the direct setTimeout with the lifecycle-aware
UseTimerService (inject UseTimerService and call its schedule/clear methods).
Ensure you still call this.formatterGetter().getCaretPosition(digitIndex) and
setCaret(el, position) inside the scheduled callback, and clear any pending
timer in the service's destroy/cleanup path.
In `@packages/phone-mask-angular/src/services/internal/useTheme.service.ts`:
- Around line 33-36: In handleThemeChange (the private arrow method
handleThemeChange in useTheme.service), remove the explicit
this.cdr.markForCheck() call because updating the signal
this.systemDark.set(event.matches) is sufficient to propagate changes; simply
delete the this.cdr.markForCheck() line so the method only sets the signal,
keeping the MediaQueryListEvent handler logic intact.
In `@packages/phone-mask-angular/src/services/usePhoneMask.service.ts`:
- Around line 60-62: The configure method currently silently returns if
this.configured is true which can hide accidental reconfiguration; update
UsePhoneMaskService.configure to detect when configure() is called more than
once (check this.configured) and either log a warning in development builds (use
process.env.NODE_ENV or an existing isDev flag) or throw an Error to fail fast,
and include the provided options and a clear message referencing configure and
this.configured so callers can diagnose mismatched reconfiguration attempts.
In `@packages/phone-mask-angular/src/services/utility/useClipboard.service.ts`:
- Line 35: The call to clearTimeout currently runs unguarded in the method
containing "if (this.resetTimer) clearTimeout(this.resetTimer);" — make it
consistent with the constructor by explicitly checking the resetTimer property
before calling clearTimeout (e.g., wrap clearTimeout(this.resetTimer) in an if
(this.resetTimer) { ... }) so the method uses the same guard pattern for the
resetTimer field in the useClipboard service.
In `@packages/phone-mask-angular/tests/unit/usePhoneMask.test.ts`:
- Around line 31-35: The test is calling mask.connect(...) twice which can
register duplicate listeners; remove the redundant call from the test setup so
only UsePhoneMaskHostComponent.ngAfterViewInit() performs the connect.
Specifically, delete or stop invoking host.mask.connect(...) inside the setup()
in usePhoneMask.test.ts (and any duplicate at the other mentioned location) so
that the only connect() invocation is the one in
UsePhoneMaskHostComponent.ngAfterViewInit(), leaving mask.connect(...) usage
single and preventing duplicate event listeners.
In `@packages/phone-mask-angular/tests/unit/useTimer.test.ts`:
- Around line 17-27: The setup() helper calls TestBed.tick() outside a fakeAsync
zone; wrap the body of setup() in Angular's fakeAsync and invoke it immediately
so TestBed.tick() runs inside the fakeAsync zone. Add an import for fakeAsync
from '@angular/core/testing', wrap the existing TestBed.configureTestingModule /
createComponent / fixture.detectChanges / TestBed.tick sequence in fakeAsync(()
=> { ... })() and return the same object (result and unmount) from the wrapped
invocation; update the setup() function and keep references to
UseTimerHostComponent and fixture/componentInstance.service unchanged.
In `@packages/phone-mask-angular/tsconfig.json`:
- Around line 8-13: The tsconfig's compilerOptions sets "inlineSources": true
but neither "sourceMap" nor "inlineSourceMap" is enabled, so either enable
source maps or remove inlineSources: update compilerOptions to either add
"sourceMap": true (for separate .map files) or "inlineSourceMap": true (for
inline maps) alongside "inlineSources": true, or remove "inlineSources": true if
you don't want source maps; refer to the "inlineSources", "sourceMap", and
"inlineSourceMap" keys in the tsconfig.json to make the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 20845425-3375-4835-91e1-a2ede4ba640f
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (86)
.gitignore.npmrcREADME.mdcommon/tests/unit/setup/tools.tseslint.config.jspackage.jsonpackages/phone-mask-angular/CHANGELOG.mdpackages/phone-mask-angular/README.mdpackages/phone-mask-angular/angular.jsonpackages/phone-mask-angular/core/ng-package.jsonpackages/phone-mask-angular/core/src/public-api.tspackages/phone-mask-angular/demo/src/app.component.tspackages/phone-mask-angular/demo/src/index.htmlpackages/phone-mask-angular/demo/src/main.tspackages/phone-mask-angular/demo/tsconfig.jsonpackages/phone-mask-angular/ng-package.jsonpackages/phone-mask-angular/package.jsonpackages/phone-mask-angular/playwright.config.tspackages/phone-mask-angular/src/components/phone-input/phone-input.component.htmlpackages/phone-mask-angular/src/components/phone-input/phone-input.component.scsspackages/phone-mask-angular/src/components/phone-input/phone-input.component.tspackages/phone-mask-angular/src/core.tspackages/phone-mask-angular/src/phone-mask.directive.tspackages/phone-mask-angular/src/phone-mask.pipe.tspackages/phone-mask-angular/src/public-api.tspackages/phone-mask-angular/src/services/internal/useCopyAction.service.tspackages/phone-mask-angular/src/services/internal/useCountry.service.tspackages/phone-mask-angular/src/services/internal/useCountrySelector.service.tspackages/phone-mask-angular/src/services/internal/useFormatter.service.tspackages/phone-mask-angular/src/services/internal/useInputHandlers.service.tspackages/phone-mask-angular/src/services/internal/useTheme.service.tspackages/phone-mask-angular/src/services/internal/useValidationHint.service.tspackages/phone-mask-angular/src/services/usePhoneMask.service.tspackages/phone-mask-angular/src/services/utility/useClipboard.service.tspackages/phone-mask-angular/src/services/utility/useTimer.service.tspackages/phone-mask-angular/src/types.tspackages/phone-mask-angular/tests/e2e/PhoneInput.spec.tspackages/phone-mask-angular/tests/e2e/UsePhoneMask.spec.tspackages/phone-mask-angular/tests/tsconfig.jsonpackages/phone-mask-angular/tests/unit/PhoneInput.test.tspackages/phone-mask-angular/tests/unit/index.test.tspackages/phone-mask-angular/tests/unit/phoneMaskDirective.test.tspackages/phone-mask-angular/tests/unit/service-pipe.test.tspackages/phone-mask-angular/tests/unit/setup/angular.tspackages/phone-mask-angular/tests/unit/setup/tools.tspackages/phone-mask-angular/tests/unit/useClipboard.test.tspackages/phone-mask-angular/tests/unit/useCopyAction.test.tspackages/phone-mask-angular/tests/unit/useCountry.test.tspackages/phone-mask-angular/tests/unit/useCountrySelector.test.tspackages/phone-mask-angular/tests/unit/useFormatter.test.tspackages/phone-mask-angular/tests/unit/useInputHandlers.test.tspackages/phone-mask-angular/tests/unit/usePhoneMask.test.tspackages/phone-mask-angular/tests/unit/useTheme.test.tspackages/phone-mask-angular/tests/unit/useTimer.test.tspackages/phone-mask-angular/tests/unit/useValidationHint.test.tspackages/phone-mask-angular/tsconfig.jsonpackages/phone-mask-angular/tsconfig.spec.jsonpackages/phone-mask-angular/vitest.angular.config.tspackages/phone-mask-react/tests/e2e/PhoneInput.spec.tspackages/phone-mask-react/tests/e2e/UsePhoneMask.spec.tspackages/phone-mask-react/tests/unit/PhoneInput.test.tsxpackages/phone-mask-react/tests/unit/index.test.tspackages/phone-mask-react/tests/unit/usePhoneMask.test.tsxpackages/phone-mask-svelte/tests/e2e/PhoneInput.spec.tspackages/phone-mask-svelte/tests/e2e/UsePhoneMask.spec.tspackages/phone-mask-svelte/tests/unit/index.test.tspackages/phone-mask-svelte/tests/unit/useCopyAction.test.tspackages/phone-mask-svelte/tests/unit/usePhoneMask.test.tspackages/phone-mask-vue/tests/e2e/PhoneInput.spec.tspackages/phone-mask-vue/tests/e2e/UsePhoneMask.spec.tspackages/phone-mask-vue/tests/unit/PhoneInput.test.tspackages/phone-mask-vue/tests/unit/index.test.tspackages/phone-mask-vue/tests/unit/usePhoneMask.test.tspackages/phone-mask/tests/tsconfig.jsonpackages/phone-mask/tests/unit/country-code-emodji.test.tspackages/phone-mask/tests/unit/country-selector.test.tspackages/phone-mask/tests/unit/data-min.test.tspackages/phone-mask/tests/unit/entries.test.tspackages/phone-mask/tests/unit/exports.test.tspackages/phone-mask/tests/unit/formatter.test.tspackages/phone-mask/tests/unit/geoip.test.tspackages/phone-mask/tests/unit/handlers.test.tspackages/phone-mask/tests/unit/utils.test.tspackages/phone-mask/vitest.config.tspnpm-workspace.yamltsconfig.json
Comment on lines
+440
to
+450
| protected setDisabled(event: Event): void { | ||
| const checked = (event.target as HTMLInputElement).checked; | ||
| this.disabled.set(checked); | ||
| this.playgroundPhone()?.setDisabledState(checked); | ||
| this.changeDetector.detectChanges(); | ||
| } | ||
|
|
||
| protected setReadonly(event: Event): void { | ||
| this.readonly.set((event.target as HTMLInputElement).checked); | ||
| this.changeDetector.detectChanges(); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Manual change detection may be unnecessary with signals.
Lines 444 and 449 call detectChanges() after updating signal-backed state. With Angular's signal integration, these manual calls should be unnecessary as signals automatically mark components for check. The call at line 444 after setDisabledState() might be defensive, but the one at line 449 (after only a signal update) appears redundant.
🧪 Consider testing without manual detection
protected setReadonly(event: Event): void {
this.readonly.set((event.target as HTMLInputElement).checked);
- this.changeDetector.detectChanges();
}Verify that change detection still works correctly, then remove unnecessary calls.
📝 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
| protected setDisabled(event: Event): void { | |
| const checked = (event.target as HTMLInputElement).checked; | |
| this.disabled.set(checked); | |
| this.playgroundPhone()?.setDisabledState(checked); | |
| this.changeDetector.detectChanges(); | |
| } | |
| protected setReadonly(event: Event): void { | |
| this.readonly.set((event.target as HTMLInputElement).checked); | |
| this.changeDetector.detectChanges(); | |
| } | |
| protected setDisabled(event: Event): void { | |
| const checked = (event.target as HTMLInputElement).checked; | |
| this.disabled.set(checked); | |
| this.playgroundPhone()?.setDisabledState(checked); | |
| this.changeDetector.detectChanges(); | |
| } | |
| protected setReadonly(event: Event): void { | |
| this.readonly.set((event.target as HTMLInputElement).checked); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/phone-mask-angular/demo/src/app.component.ts` around lines 440 -
450, Remove the unnecessary manual change detection: in the setReadonly method,
delete the call to this.changeDetector.detectChanges(); for setDisabled, try
removing the final this.changeDetector.detectChanges() after updating the signal
and calling this.playgroundPhone()?.setDisabledState(checked) (keep it only if
you find a regression in rendering), then run the demo/tests to verify UI
updates; the relevant symbols are the setDisabled and setReadonly methods,
playgroundPhone()?.setDisabledState and this.changeDetector.detectChanges.
| import { bootstrapApplication } from '@angular/platform-browser'; | ||
| import { AppComponent } from './app.component'; | ||
|
|
||
| bootstrapApplication(AppComponent); |
There was a problem hiding this comment.
Catch bootstrap errors to surface failures.
bootstrapApplication returns a Promise that may reject (e.g., on provider misconfiguration). Without a .catch, errors only appear as an unhandled rejection in dev tools. Matches the Angular CLI default template:
🛡️ Proposed fix
-bootstrapApplication(AppComponent);
+bootstrapApplication(AppComponent).catch((err) => console.error(err));📝 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
| bootstrapApplication(AppComponent); | |
| bootstrapApplication(AppComponent).catch((err) => console.error(err)); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/phone-mask-angular/demo/src/main.ts` at line 4,
bootstrapApplication(AppComponent) is not handling promise rejections; update
the call to catch and surface bootstrap failures by appending a rejection
handler that logs the error (and exits if desired). Specifically add a .catch
handler on the bootstrapApplication(AppComponent) promise to call console.error
(or your logger) with a clear message and the error object so
provider/configuration errors are visible at startup; reference
bootstrapApplication and AppComponent to locate the call to update.
| @@ -0,0 +1,9 @@ | |||
| { | |||
| "$schema": "./node_modules/ng-packagr/ng-package.schema.json", | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if ng-packagr build works and verify schema path conventions
cd packages/phone-mask-angular
# Verify the schema file exists at the specified path
if [ -f "./node_modules/ng-packagr/ng-package.schema.json" ]; then
echo "✓ Schema file found at ./node_modules/ng-packagr/ng-package.schema.json"
else
echo "✗ Schema file NOT found at ./node_modules/ng-packagr/ng-package.schema.json"
fi
# Check how other Angular packages typically reference the schema
echo ""
echo "Common schema path patterns in Angular ecosystem:"
rg --type=json -C2 '\$schema.*ng-package' -g 'ng-package.json' || echo "No other ng-package.json files found for comparison"Repository: DeSource-Labs/phone-mask
Length of output: 374
🏁 Script executed:
# Check repository structure and monorepo setup
echo "=== Repository root structure ==="
ls -la | head -20
echo -e "\n=== Check for package.json files ==="
find . -name "package.json" -type f | head -20
echo -e "\n=== Check packages/phone-mask-angular directory ==="
ls -la packages/phone-mask-angular/ 2>/dev/null | head -20
echo -e "\n=== Check for node_modules locations ==="
find . -name "node_modules" -type d 2>/dev/null | head -10
echo -e "\n=== Contents of packages/phone-mask-angular/ng-package.json ==="
cat packages/phone-mask-angular/ng-package.json 2>/dev/nullRepository: DeSource-Labs/phone-mask
Length of output: 5203
🏁 Script executed:
# Check root package.json to understand monorepo structure
echo "=== Root package.json ==="
head -30 package.json
echo -e "\n=== Check for ng-packagr in root package.json ==="
cat package.json | grep -A5 -B5 "ng-packagr" || echo "Not found in root"Repository: DeSource-Labs/phone-mask
Length of output: 1591
🏁 Script executed:
# Look for CI/build documentation
echo "=== Check for CI workflows ==="
ls -la .github/workflows/ 2>/dev/null || echo "No .github/workflows directory"
echo -e "\n=== Check for build scripts in package.json ==="
grep -E '"build|"ng-' package.json || echo "No build scripts found"Repository: DeSource-Labs/phone-mask
Length of output: 637
🌐 Web query:
ng-packagr schema path configuration monorepo
💡 Result:
In an ng-packagr monorepo, the ng-package configuration (“schema path”) you normally reference is the JSON schema file at: - ng-packagr provides an ng-package.json schema at ng-packagr/ng-package.schema.json (the schema that $schema points to). [1][2] How this relates to “schema path configuration” 1) In each library’s ng-package.json, you can set the $schema property to enable IDE validation/autocompletion. ng-packagr’s README shows $schema referencing ./node_modules/ng-packagr/ng-package.schema.json. [3] Example (library root folder): { "$schema": "./node_modules/ng-packagr/ng-package.schema.json", "lib": { "entryFile": "public_api.ts" } } This is the expected pattern for per-library configuration in a monorepo; ng-packagr supports one JSON configuration file per project and multiple libraries are handled via one ng-package.json per project. [4] 2) If you’re using package.json-based configuration (via the ngPackage field), ng-packagr also supports an IDE schema reference for that custom ngPackage property by setting $schema in the library’s package.json to ./node_modules/ng-packagr/package.schema.json. [5][6] Monorepo + multiple libraries: where the paths resolve - ng-packagr resolves paths relative to the location of the ng-package.json file (i.e., each library directory is treated as its own configuration base). [7] Practical takeaway - For the “schema path” question in a monorepo, point $schema at ./node_modules/ng-packagr/ng-package.schema.json inside each library’s ng-package.json (or ./node_modules/ng-packagr/package.schema.json inside a library package.json that uses the ngPackage field). [3][7][5][6]
Citations:
- 1: https://www.unpkg.com/browse/ng-packagr@12.0.3/ng-package.schema.json
- 2: https://github.com/ng-packagr/ng-packagr/blob/master/src/ng-package.schema.json
- 3: https://github.com/ng-packagr/ng-packagr/blob/main/README.md
- 4: https://github.com/ng-packagr/ng-packagr/blob/HEAD/docs/DESIGN.md
- 5: feat: add json schema for
package.jsonwith customngPackageproperty ng-packagr/ng-packagr#173 - 6: ng-packagr/ng-packagr@dd85fd2
- 7: https://github.com/ng-packagr/ng-packagr/blob/0fe359eb01c7cbc4c408811daba4d94450cbed71/README.md
🏁 Script executed:
# Check if ng-packagr is installed at root level and in the package
echo "=== Check for ng-packagr in root devDependencies ==="
grep "ng-packagr" package.json
echo -e "\n=== Check for ng-packagr in packages/phone-mask-angular/package.json ==="
grep "ng-packagr" packages/phone-mask-angular/package.json
echo -e "\n=== Check where ng-packagr is actually located ==="
find node_modules -maxdepth 2 -name "ng-packagr" -type d 2>/dev/null | head -5
echo -e "\n=== Check if ng-packagr schema file exists at root ==="
if [ -f "node_modules/ng-packagr/ng-package.schema.json" ]; then
echo "✓ Schema exists at: node_modules/ng-packagr/ng-package.schema.json"
else
echo "✗ Schema NOT found at: node_modules/ng-packagr/ng-package.schema.json"
fi
echo -e "\n=== Check packages/phone-mask-angular build script ==="
grep -A2 '"build"' packages/phone-mask-angular/package.jsonRepository: DeSource-Labs/phone-mask
Length of output: 761
🏁 Script executed:
# Check CI workflow for build status
echo "=== Check coverage.yml for ng build steps ==="
cat .github/workflows/coverage.yml | grep -A10 "phone-mask-angular\|ng build\|ng-packagr" || echo "No direct reference found"
echo -e "\n=== Check what build command is used ==="
cat .github/workflows/coverage.yml | head -50Repository: DeSource-Labs/phone-mask
Length of output: 1582
🏁 Script executed:
# Let's check if the build actually succeeds by looking at CI workflow
echo "=== Check release.yml which would have build steps ==="
cat .github/workflows/release.yml | grep -A30 "steps:"
echo -e "\n=== Check if there are any known build issues ==="
git log --oneline -20 | head -20
echo -e "\n=== Check pnpm lockfile for ng-packagr ==="
grep -m3 "ng-packagr" pnpm-lock.yaml 2>/dev/null | head -10Repository: DeSource-Labs/phone-mask
Length of output: 2056
🏁 Script executed:
# Verify what resolves from the schema $schema property
# The $schema is typically just for IDE validation, not runtime functionality
echo "=== Checking ng-packagr CLI behavior ==="
cd packages/phone-mask-angular
cat package.json | grep -A10 "devDependencies\|dependencies"Repository: DeSource-Labs/phone-mask
Length of output: 585
🌐 Web query:
ng-packagr schema path monorepo pnpm ./node_modules relative path
💡 Result:
In ng-packagr setups, including monorepos using pnpm, the standard relative path for the JSON schema in ng-package.json (or package.json under "ngPackage") is "./node_modules/ng-packagr/ng-package.schema.json" [1][2][3]. This points to the schema file installed via npm/pnpm/yarn in the local project's node_modules [4]. For monorepos (e.g., with pnpm workspaces), place an ng-package.json in each library's directory (e.g., libs/my-lib/ng-package.json). Use a relative path adjusted to the library's depth from the root node_modules, such as "../../node_modules/ng-packagr/ng-package.schema.json" if the library is two levels deep [5][6]. Example for a library at libs/my-lib/ng-package.json: { "$schema": "../../node_modules/ng-packagr/ng-package.schema.json", "dest": "../../dist/my-lib", "lib": { "entryFile": "src/public-api.ts" } } [5][6] pnpm works with ng-packagr in monorepos as it creates symlinks in node_modules compatible with ng-packagr (resolved in v13+) [7]. Use pnpm workspace:* for cross-library deps, build in topological order, and map tsconfig paths to dist/ outputs (not sources) to avoid rootDir errors [8][9]. ng-packagr does not natively read tsconfig paths for external libs; override via CLI options or Angular CLI project config [10][11].
Citations:
- 1: feat: add json schema for
package.jsonwith customngPackageproperty ng-packagr/ng-packagr#173 - 2: https://www.npmjs.com/package/ng-packagr
- 3: https://github.com/ng-packagr/ng-packagr
- 4: https://www.unpkg.com/browse/ng-packagr@12.0.3/ng-package.schema.json
- 5: Building a library with dependent libraries defined via paths ng-packagr/ng-packagr#1264
- 6: https://stackoverflow.com/questions/67225441/angular-mono-repo-library-making-using-ng-packagr
- 7: Add PNPM support ng-packagr/ng-packagr#1595
- 8: Cannot build library that depends on other library in monorepo ng-packagr/ng-packagr#2231
- 9: https://stackoverflow.com/questions/73376408/typescript-pnpm-monorepo-path-aliases-conflicts-between-local-dependencies-duri
- 10: https://stackoverflow.com/questions/50799338/using-ng-packagr-it-doesnt-recognise-my-tsconfig-paths
- 11: https://github.com/ng-packagr/ng-packagr/blob/main/docs/override-tsconfig.md
Use the correct relative path for IDE schema validation in this pnpm monorepo.
The schema path should be "../../node_modules/ng-packagr/ng-package.schema.json" (two levels up from packages/phone-mask-angular/) to match the monorepo structure where node_modules is at the root. The current path "./node_modules/..." won't resolve correctly for IDE validation. Note: This affects IDE autocomplete and validation only—ng-packagr's CLI doesn't use the $schema property, so builds still succeed.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/phone-mask-angular/ng-package.json` at line 2, Update the "$schema"
value in ng-package.json to point to the repo root node_modules for IDE
validation: replace the current
"./node_modules/ng-packagr/ng-package.schema.json" reference with
"../../node_modules/ng-packagr/ng-package.schema.json" so IDEs can resolve the
schema for the packages/phone-mask-angular project.
Comment on lines
+65
to
+74
| readonly countryInput = input<CountryKey | string | undefined>(undefined, { alias: 'country' }); | ||
| readonly detectInput = input(true, { | ||
| alias: 'detect', | ||
| transform: booleanAttribute | ||
| }); | ||
| readonly localeInput = input<string | undefined>(undefined, { alias: 'locale' }); | ||
| readonly size = input<Size>('normal'); | ||
| readonly theme = input<Theme>('auto'); | ||
| readonly disabledInput = input(false, { alias: 'disabled', transform: booleanAttribute }); | ||
| readonly readOnlyInput = input(false, { alias: 'readonly', transform: booleanAttribute }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚖️ Poor tradeoff
Consider renaming internal properties instead of aliasing inputs.
Lines 65, 67, 70, 73-74 use input aliasing to avoid naming conflicts with exposed properties (e.g., countryInput aliased as country to avoid conflict with this.country at line 117). While this works, Angular style guide discourages input aliasing.
Consider renaming the exposed computed properties instead:
country→selectedCountryorcurrentCountrylocale→currentLocale
This eliminates the need for aliases while improving clarity.
As per static analysis hints, input aliasing is discouraged by Angular style guide.
🧰 Tools
🪛 GitHub Check: SonarCloud Code Analysis
[failure] 70-70: Input bindings should not be aliased (https://angular.dev/guide/components/inputs#choosing-input-names)
[failure] 67-67: Input bindings should not be aliased (https://angular.dev/guide/components/inputs#choosing-input-names)
[failure] 74-74: Input bindings should not be aliased (https://angular.dev/guide/components/inputs#choosing-input-names)
[failure] 73-73: Input bindings should not be aliased (https://angular.dev/guide/components/inputs#choosing-input-names)
[failure] 65-65: Input bindings should not be aliased (https://angular.dev/guide/components/inputs#choosing-input-names)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@packages/phone-mask-angular/src/components/phone-input/phone-input.component.ts`
around lines 65 - 74, The component is using input aliasing (countryInput,
localeInput, disabledInput, readOnlyInput etc.) to avoid name conflicts with
exposed computed properties; rename the exposed computed properties instead and
restore clean input names: rename the computed property country →
selectedCountry (and any references to this.country) and locale → currentLocale
(and references to this.locale), then rename the input fields back to their
public names (e.g., countryInput → country, localeInput → locale) and remove the
alias option from their input(...) calls (also remove alias for
disabled/readOnly if present), and update any template/consumer references to
the new computed names so aliasing is no longer required.
Comment on lines
+87
to
+90
| readonly focused = output<FocusEvent>({ alias: 'focus' }); | ||
| readonly blurred = output<FocusEvent>({ alias: 'blur' }); | ||
| readonly copiedValue = output<string>({ alias: 'copy' }); | ||
| readonly cleared = output<void>({ alias: 'clear' }); |
There was a problem hiding this comment.
Output aliases collide with native DOM events.
The aliases focus, blur, and copy (lines 87-89) match native DOM event names. In templates, (focus)="handler($event)" will bind to the component output rather than the native event, which can cause confusion and makes it impossible to listen to actual focus/blur events on the component host without workarounds.
Consider either:
- Removing aliases and using descriptive names (
focused,blurred,copied) - Using non-colliding alias names (
phoneFocus,phoneBlur,phoneCopy)
As per static analysis hints, Angular style guide discourages output aliasing and warns against naming outputs after DOM events.
♻️ Recommended fix: remove colliding aliases
- readonly focused = output<FocusEvent>({ alias: 'focus' });
- readonly blurred = output<FocusEvent>({ alias: 'blur' });
- readonly copiedValue = output<string>({ alias: 'copy' });
- readonly cleared = output<void>({ alias: 'clear' });
+ readonly focused = output<FocusEvent>();
+ readonly blurred = output<FocusEvent>();
+ readonly copied = output<string>();
+ readonly cleared = output<void>();Update templates to use (focused), (blurred), (copied), (cleared) instead of aliased names.
📝 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
| readonly focused = output<FocusEvent>({ alias: 'focus' }); | |
| readonly blurred = output<FocusEvent>({ alias: 'blur' }); | |
| readonly copiedValue = output<string>({ alias: 'copy' }); | |
| readonly cleared = output<void>({ alias: 'clear' }); | |
| readonly focused = output<FocusEvent>(); | |
| readonly blurred = output<FocusEvent>(); | |
| readonly copied = output<string>(); | |
| readonly cleared = output<void>(); |
🧰 Tools
🪛 GitHub Check: SonarCloud Code Analysis
[failure] 89-89: Output bindings, including aliases, should not be named as standard DOM events
[failure] 90-90: Output bindings should not be aliased (https://angular.dev/guide/components/outputs#choosing-event-names)
[failure] 89-89: Output bindings should not be aliased (https://angular.dev/guide/components/outputs#choosing-event-names)
[failure] 87-87: Output bindings should not be aliased (https://angular.dev/guide/components/outputs#choosing-event-names)
[failure] 87-87: Output bindings, including aliases, should not be named as standard DOM events
[failure] 88-88: Output bindings, including aliases, should not be named as standard DOM events
[failure] 88-88: Output bindings should not be aliased (https://angular.dev/guide/components/outputs#choosing-event-names)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@packages/phone-mask-angular/src/components/phone-input/phone-input.component.ts`
around lines 87 - 90, The component defines outputs with aliases that collide
with native DOM events—specifically the outputs declared as readonly focused =
output<FocusEvent>({ alias: 'focus' }), readonly blurred = output<FocusEvent>({
alias: 'blur' }), and readonly copiedValue = output<string>({ alias: 'copy'
})—so change these to non-colliding names by removing the alias or using unique
aliases (e.g., remove alias so the outputs are exposed as focused, blurred,
copied or use phoneFocus/phoneBlur/phoneCopy), and update any templates that
currently bind to (focus)/(blur)/(copy) to instead bind to
(focused)/(blurred)/(copied) or the chosen unique names; do the same check for
cleared to ensure its alias (if any) doesn't collide.
Comment on lines
+60
to
+62
| configure(options: UsePhoneMaskOptions): void { | ||
| if (this.configured) return; | ||
| this.configured = true; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Consider warning on reconfiguration attempts.
The silent early return may hide bugs if configure() is accidentally called multiple times with different options. Consider logging a warning in development mode or throwing an error to make misconfiguration more visible.
💡 Suggested enhancement
configure(options: UsePhoneMaskOptions): void {
- if (this.configured) return;
+ if (this.configured) {
+ console.warn('UsePhoneMaskService: configure() called multiple times. Ignoring subsequent calls.');
+ return;
+ }
this.configured = true;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/phone-mask-angular/src/services/usePhoneMask.service.ts` around
lines 60 - 62, The configure method currently silently returns if
this.configured is true which can hide accidental reconfiguration; update
UsePhoneMaskService.configure to detect when configure() is called more than
once (check this.configured) and either log a warning in development builds (use
process.env.NODE_ENV or an existing isDev flag) or throw an Error to fail fast,
and include the provided options and a clear message referencing configure and
this.configured so callers can diagnose mismatched reconfiguration attempts.
|
|
||
| this.copied.set(true); | ||
|
|
||
| if (this.resetTimer) clearTimeout(this.resetTimer); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Optional: Guard resetTimer before clearing.
While not strictly necessary (clearTimeout handles undefined), checking this.resetTimer before clearing matches the pattern used in the constructor (line 17) and is a minor optimization.
♻️ Optional consistency improvement
- if (this.resetTimer) clearTimeout(this.resetTimer);
+ if (this.resetTimer) {
+ clearTimeout(this.resetTimer);
+ this.resetTimer = undefined;
+ }📝 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
| if (this.resetTimer) clearTimeout(this.resetTimer); | |
| if (this.resetTimer) { | |
| clearTimeout(this.resetTimer); | |
| this.resetTimer = undefined; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/phone-mask-angular/src/services/utility/useClipboard.service.ts` at
line 35, The call to clearTimeout currently runs unguarded in the method
containing "if (this.resetTimer) clearTimeout(this.resetTimer);" — make it
consistent with the constructor by explicitly checking the resetTimer property
before calling clearTimeout (e.g., wrap clearTimeout(this.resetTimer) in an if
(this.resetTimer) { ... }) so the method uses the same guard pattern for the
resetTimer field in the useClipboard service.
Comment on lines
+31
to
+35
| ngAfterViewInit(): void { | ||
| if (this.attachRef) { | ||
| this.mask.connect(this.inputRef()?.nativeElement ?? null); | ||
| } | ||
| } |
There was a problem hiding this comment.
Avoid double connect() in test setup.
UsePhoneMaskHostComponent.ngAfterViewInit() already connects the input. Calling host.mask.connect(...) again in setup() can register duplicate listeners and make event assertions flaky if connect() is not idempotent.
Suggested fix
host.value.set(initialValue);
result.detectChanges();
- if (attachRef) host.mask.connect(result.container.querySelector('input'));
await tools.act(async () => {});Also applies to: 61-61
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/phone-mask-angular/tests/unit/usePhoneMask.test.ts` around lines 31
- 35, The test is calling mask.connect(...) twice which can register duplicate
listeners; remove the redundant call from the test setup so only
UsePhoneMaskHostComponent.ngAfterViewInit() performs the connect. Specifically,
delete or stop invoking host.mask.connect(...) inside the setup() in
usePhoneMask.test.ts (and any duplicate at the other mentioned location) so that
the only connect() invocation is the one in
UsePhoneMaskHostComponent.ngAfterViewInit(), leaving mask.connect(...) usage
single and preventing duplicate event listeners.
Comment on lines
+17
to
+27
| function setup() { | ||
| TestBed.configureTestingModule({ imports: [UseTimerHostComponent] }); | ||
| const fixture = TestBed.createComponent(UseTimerHostComponent); | ||
| fixture.detectChanges(); | ||
| TestBed.tick(); | ||
|
|
||
| return { | ||
| result: fixture.componentInstance.service, | ||
| unmount: () => fixture.destroy() | ||
| }; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Search for fakeAsync usage patterns in other Angular test files to verify the testing approach.
# Check if other test files use fakeAsync with TestBed.tick()
rg -n 'fakeAsync|waitForAsync' --type ts -g 'packages/phone-mask-angular/tests/**/*.test.ts'
# Check if the shared test harness might handle async zones
rg -n 'fakeAsync|waitForAsync|tick' --type ts -g 'common/tests/unit/useTimer*'Repository: DeSource-Labs/phone-mask
Length of output: 50
🏁 Script executed:
#!/bin/bash
# Find the test file and check if it exists
fd -u 'useTimer.test.ts' packages/
# Also search for any TestBed.tick() usage in the codebase
rg -n 'TestBed\.tick' --type ts packages/phone-mask-angular/Repository: DeSource-Labs/phone-mask
Length of output: 1432
🏁 Script executed:
#!/bin/bash
# Read the test file to understand the context
cat -n packages/phone-mask-angular/tests/unit/useTimer.test.ts
echo "---"
# Read the tools/setup file to check if it handles async zones
cat -n packages/phone-mask-angular/tests/unit/setup/tools.tsRepository: DeSource-Labs/phone-mask
Length of output: 1856
🏁 Script executed:
#!/bin/bash
# Find the shared testUseTimer function
fd -u 'useTimer' common/tests/unit/ --type f
echo "---"
# Read the testUseTimer implementation
cat -n common/tests/unit/useTimer.ts | head -100Repository: DeSource-Labs/phone-mask
Length of output: 3282
🏁 Script executed:
#!/bin/bash
# Check for vitest configuration
fd -u 'vitest.config|vite.config' . --type f
echo "---"
# Check for test setup files that might configure async zones
fd -u 'setup|config' packages/phone-mask-angular/tests --type f
echo "---"
# Look for any fakeAsync or waitForAsync wrappers around tests
rg -n 'fakeAsync|waitForAsync|beforeEach|afterEach' packages/phone-mask-angular/tests/unit/setup --type tsRepository: DeSource-Labs/phone-mask
Length of output: 557
🏁 Script executed:
#!/bin/bash
# Check the Angular vitest configuration
fd -u 'vitest.config' packages/phone-mask-angular --type f
echo "---"
# Read the Angular vitest config
cat -n packages/phone-mask-angular/vitest.config.ts 2>/dev/null || echo "Not found in angular package"
echo "---"
# Check the main vitest config that might apply globally
cat -n packages/phone-mask/vitest.config.ts
echo "---"
# Search for any setupFiles or global setup configurations
rg -n 'setupFiles|setupTestEnv|globals' packages/phone-mask/vitest.config.tsRepository: DeSource-Labs/phone-mask
Length of output: 780
🏁 Script executed:
#!/bin/bash
# Check if TestBed is initialized with async wrapper anywhere
rg -n 'initTestEnvironment|TestBedInit|fakeAsync|waitForAsync' packages/phone-mask-angular/tests --type ts
echo "---"
# Check if there's a global test setup file
find packages/phone-mask-angular/tests -name 'setup.*' -o -name 'vitest.*' -o -name '*.setup.*'
echo "---"
# Look for test environment configuration specific to Angular
cat -n packages/phone-mask-angular/tests/tsconfig.jsonRepository: DeSource-Labs/phone-mask
Length of output: 745
🏁 Script executed:
#!/bin/bash
# Read the angular.ts setup file to see how TestBed is configured
cat -n packages/phone-mask-angular/tests/unit/setup/angular.ts
echo "---"
# Check if this file is imported globally in test setup
rg -n 'setup/angular' packages/phone-mask-angular/tests --type tsRepository: DeSource-Labs/phone-mask
Length of output: 513
🏁 Script executed:
#!/bin/bash
# Check if the angular.ts setup is imported in test files
rg -n "import.*setup/angular|from.*setup/angular" packages/phone-mask-angular/tests --type ts
echo "---"
# Check if setup/tools is actually exporting anything related to Angular async zones
cat -n packages/phone-mask-angular/tests/unit/setup/tools.ts
echo "---"
# Search for how setup() function is actually called in context
rg -B5 -A5 "testUseTimer\(setup" packages/phone-mask-angular/tests/unit/useTimer.test.tsRepository: DeSource-Labs/phone-mask
Length of output: 966
🏁 Script executed:
#!/bin/bash
# Check if there's any test configuration that automatically wraps with fakeAsync
rg -n 'beforeEach|afterEach|beforeAll|afterAll' packages/phone-mask-angular/tests/unit --type ts -A3
echo "---"
# Search for any wrapper or utility that might handle the async zone
rg -n 'fakeAsync|FakeAsyncTestZoneSpec|AsyncTestZoneSpec' packages/phone-mask-angular --type ts
echo "---"
# Check if vitest or testing-library has special handling
rg -n 'TestBed' packages/phone-mask-angular/tests/unit/setup --type ts -B2 -A2Repository: DeSource-Labs/phone-mask
Length of output: 2554
🏁 Script executed:
#!/bin/bash
# Check if angular.ts is imported as a global setupFile or side effect
grep -r "setup/angular" packages/phone-mask-angular --type-list ts
echo "---"
# Check package.json or vitest config for any setupFiles configuration
cat -n packages/phone-mask-angular/package.json | grep -A10 -B10 "test\|vitest" || echo "checking vite config"
echo "---"
# Look in root package.json for global test setup
cat -n package.json | grep -A20 "test\|vitest" | head -30Repository: DeSource-Labs/phone-mask
Length of output: 3456
🏁 Script executed:
#!/bin/bash
# Check if angular setup file is imported globally in test files
grep -r "setup/angular" packages/phone-mask-angular/tests --include="*.ts"
echo "---"
# Check the angular.json configuration for test setup
cat -n angular.json 2>/dev/null | head -50
echo "---"
# Look for karma.conf or test runner configuration
find packages/phone-mask-angular -name "karma.conf*" -o -name "angular.json" | xargs ls -la 2>/dev/nullRepository: DeSource-Labs/phone-mask
Length of output: 166
🏁 Script executed:
#!/bin/bash
# Read the angular.json file to see test configuration
cat -n packages/phone-mask-angular/angular.json
echo "---"
# Check if there's a vitest.workspace.ts or similar that provides global setup
find packages/phone-mask-angular -name "*.ts" -o -name "*.json" | xargs grep -l "setupFiles\|setupTestEnv" 2>/dev/null | head -5
echo "---"
# Verify if the setup/angular.ts file is actually imported anywhere
rg -r "import.*angular\.ts|from.*angular\.ts" packages/phone-mask-angular --type tsRepository: DeSource-Labs/phone-mask
Length of output: 2175
🏁 Script executed:
#!/bin/bash
# Read the vitest angular config that's referenced in angular.json
cat -n packages/phone-mask-angular/vitest.angular.config.ts 2>/dev/null || echo "File not found"
echo "---"
# Check if the file exists with any name variation
fd -u 'vitest.*config' packages/phone-mask-angular --type f
echo "---"
# Search for any documentation or comments about TestBed.tick() behavior
rg -n 'TestBed\.tick|fakeAsync|waitForAsync' packages/phone-mask-angular --type ts -B2 -A2 | head -60Repository: DeSource-Labs/phone-mask
Length of output: 5490
🏁 Script executed:
#!/bin/bash
# Check if there are any actual test execution results or error logs
find packages/phone-mask-angular -name "test-results" -o -name "coverage" -o -name ".test-results" -type d
echo "---"
# Look for any GitHub Actions or CI configuration that runs tests
find . -name "*.yml" -o -name "*.yaml" | xargs grep -l "test:unit\|vitest" 2>/dev/null | head -3
echo "---"
# Check if there's any wrapper utility that might handle fakeAsync
rg -n 'export.*setup|export.*fakeAsync|function setup' packages/phone-mask-angular/tests/unit/setup --type ts -A3Repository: DeSource-Labs/phone-mask
Length of output: 209
🏁 Script executed:
#!/bin/bash
# Check the coverage workflow to see how tests are run
cat -n ./.github/workflows/coverage.yml
echo "---"
# Check if TestBed.tick() without fakeAsync is actually documented to work
# Look for any Angular version constraints or special handling in dependencies
cat -n packages/phone-mask-angular/package.json | grep -A5 -B5 "angular"Repository: DeSource-Labs/phone-mask
Length of output: 9188
🏁 Script executed:
#!/bin/bash
# Check if there are any recent changes to test files related to this issue
git log --oneline -n 20 packages/phone-mask-angular/tests/unit/useTimer.test.ts 2>/dev/null | head -10
echo "---"
# Search for any comments or documentation about this pattern
rg -n 'tick|async|fakeAsync' packages/phone-mask-angular/tests/unit/useTimer.test.ts
echo "---"
# Check the common testUseTimer to see full implementation and if it wraps with fakeAsync
cat -n common/tests/unit/useTimer.tsRepository: DeSource-Labs/phone-mask
Length of output: 5445
Wrap TestBed.tick() in setup() with fakeAsync zone.
TestBed.tick() at line 21 is called outside a fakeAsync() or waitForAsync() zone. According to Angular testing best practices, tick() requires an async zone wrapper to properly advance the virtual clock and flush pending timers.
Recommended fix
import { fakeAsync } from '@angular/core/testing';
function setup() {
return fakeAsync(() => {
TestBed.configureTestingModule({ imports: [UseTimerHostComponent] });
const fixture = TestBed.createComponent(UseTimerHostComponent);
fixture.detectChanges();
TestBed.tick();
return {
result: fixture.componentInstance.service,
unmount: () => fixture.destroy()
};
})();
}This pattern is used throughout the test suite (in useValidationHint.test.ts, useTheme.test.ts, useFormatter.test.ts, etc.), so consider applying this fix consistently across all setup functions.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/phone-mask-angular/tests/unit/useTimer.test.ts` around lines 17 -
27, The setup() helper calls TestBed.tick() outside a fakeAsync zone; wrap the
body of setup() in Angular's fakeAsync and invoke it immediately so
TestBed.tick() runs inside the fakeAsync zone. Add an import for fakeAsync from
'@angular/core/testing', wrap the existing TestBed.configureTestingModule /
createComponent / fixture.detectChanges / TestBed.tick sequence in fakeAsync(()
=> { ... })() and return the same object (result and unmount) from the wrapped
invocation; update the setup() function and keep references to
UseTimerHostComponent and fixture/componentInstance.service unchanged.
Comment on lines
+8
to
+13
| "declaration": true, | ||
| "declarationMap": true, | ||
| "emitDeclarationOnly": false, | ||
| "experimentalDecorators": true, | ||
| "inlineSources": true, | ||
| "outDir": "dist/out-tsc", |
There was a problem hiding this comment.
inlineSources requires source maps to be enabled.
Line 13 sets "inlineSources": true, which embeds source code into source map files. However, neither sourceMap nor inlineSourceMap is enabled in compilerOptions, so this setting currently has no effect.
If debugging support is desired, add either:
"sourceMap": true(separate .map files), or"inlineSourceMap": true(inline maps in .js files)
If source maps aren't needed for the library build, remove "inlineSources": true.
🗺️ Proposed fix to enable inline source maps
"composite": false,
"declaration": true,
"declarationMap": true,
"emitDeclarationOnly": false,
"experimentalDecorators": true,
+ "inlineSourceMap": true,
"inlineSources": true,
"outDir": "dist/out-tsc",📝 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
| "declaration": true, | |
| "declarationMap": true, | |
| "emitDeclarationOnly": false, | |
| "experimentalDecorators": true, | |
| "inlineSources": true, | |
| "outDir": "dist/out-tsc", | |
| "declaration": true, | |
| "declarationMap": true, | |
| "emitDeclarationOnly": false, | |
| "experimentalDecorators": true, | |
| "inlineSourceMap": true, | |
| "inlineSources": true, | |
| "outDir": "dist/out-tsc", |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/phone-mask-angular/tsconfig.json` around lines 8 - 13, The
tsconfig's compilerOptions sets "inlineSources": true but neither "sourceMap"
nor "inlineSourceMap" is enabled, so either enable source maps or remove
inlineSources: update compilerOptions to either add "sourceMap": true (for
separate .map files) or "inlineSourceMap": true (for inline maps) alongside
"inlineSources": true, or remove "inlineSources": true if you don't want source
maps; refer to the "inlineSources", "sourceMap", and "inlineSourceMap" keys in
the tsconfig.json to make the change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
What does this PR do?
@desource/phone-mask-angular, an Angular package for the phone mask libraryPhoneInputComponent,PhoneMaskDirective,PhoneMaskPipe, andUsePhoneMaskService@desource/phone-mask-angular/coreentry point for core/kit re-exportsWhy is this change needed?
Type of Change
Testing
pnpm --filter @desource/phone-mask-angular lintpnpm --filter @desource/phone-mask-angular typecheckpnpm --filter @desource/phone-mask-angular test:unitpnpm --filter @desource/phone-mask-angular buildpnpm buildpnpm test:unit:coverageScreenshots (if applicable)
Checklist
Manual Coverage (Optional)
Maintainers only (
write/maintain/adminaccess): open the workflow, clickRun workflow, and setpr_numberto this PR number to post/update a coverage comment on this PRSummary by CodeRabbit
New Features
Documentation
@desource/phone-mask-angularpackage documentation with installation and usage examples.Chores