Cu 86ewn0hjy upgrade vue3 node 18+ ernesto vivar#18
Cu 86ewn0hjy upgrade vue3 node 18+ ernesto vivar#18ernestovivar wants to merge 9 commits intomainfrom
Conversation
- Set module type for ES module support - Update Vue dependency to version 3.5.28 - Add type-checking script using vue-tsc - Modify build script to include type-checking - Update devDependencies for TypeScript, Vite, and ESLint packages
- Updated devDependencies in package.json, including jsdom and eslint plugins. - Added new dependencies for CSS handling and color parsing. - Changed event listener name from 'tawkOnChatHidden' to 'tawkChatHidden' in TawkMessenger class. - Refactored tests to reflect the updated event listener and improved structure for listener function tests.
|
Task linked: CU-86ewn0hjy Upgrade vue3 |
WalkthroughThe PR modernizes the Tawk Messenger Vue 3 plugin by migrating from JavaScript to TypeScript, implementing a flat ESLint configuration format, adding comprehensive type definitions, and updating build tooling with TypeScript support. The core widget functionality remains unchanged while gaining explicit type safety throughout. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/utils/widget.ts (1)
40-41:⚠️ Potential issue | 🟡 MinorPotential runtime crash if no
<script>tags exist.
getElementsByTagName('script')[0]returnsundefinedin environments with no existing script elements (e.g., bare jsdom setup). Calling.parentNodeonundefinedthrows immediately. Additionally,parentNodecan theoretically benull. Both are masked bystrict: falsein tsconfig.🛡️ Suggested guard
- const firstScript = document.getElementsByTagName('script')[0]; - firstScript.parentNode.insertBefore(script, firstScript); + const firstScript = document.getElementsByTagName('script')[0]; + if (firstScript?.parentNode) { + firstScript.parentNode.insertBefore(script, firstScript); + } else { + document.head.appendChild(script); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/widget.ts` around lines 40 - 41, The current insertion uses getElementsByTagName('script')[0] (assigned to firstScript) and then calls firstScript.parentNode.insertBefore(script, firstScript), which can crash if firstScript is undefined or parentNode is null; change the logic in the widget initialization to check whether firstScript exists and has a parentNode and, if not, fall back to a safe container (e.g., document.head, document.getElementsByTagName('head')[0], or document.body) and use appendChild/insertBefore on that container instead of assuming firstScript.parentNode is present; update the code paths that reference firstScript and insertBefore(script, firstScript) so they use the guarded container to avoid runtime errors in environments without script tags.package.json (1)
19-23:⚠️ Potential issue | 🟡 MinorStale repository reference to
vue-2.Line 22 has
"repository": "tawk/tawk-messenger-vue-2"nested inside therepositoryobject. This appears to be a leftover from the vue-2 package and should be corrected or removed.🐛 Suggested fix
"repository": { "type": "git", - "url": "git+https://github.com/tawk/tawk-messenger-vue-3.git", - "repository": "tawk/tawk-messenger-vue-2" + "url": "git+https://github.com/tawk/tawk-messenger-vue-3.git" },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` around lines 19 - 23, The package.json contains a stale nested "repository" field with the value "tawk/tawk-messenger-vue-2"; update or remove that nested key so the repository metadata is correct (e.g., remove the nested "repository" property or change its value to "tawk/tawk-messenger-vue-3") by editing the "repository" object in package.json where the keys "type", "url", and the incorrect "repository" currently appear.
🧹 Nitpick comments (8)
tests/units/features.spec.js (1)
51-189: Optional: reset mock state between listener tests
window.addEventListeneris asserted across ~19 independentitblocks without abeforeEachreset. Currently this is safe because each test checks a unique event name so accumulated call history doesn't produce false positives. However, as tests grow or event names change, the lack of reset could allow stale calls to mask regressions.🧹 Optional: add mock reset before each listener test
+import { describe, it, expect, beforeEach, vi } from 'vitest';describe('Should provide listeners functions', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + it('Should add tawkLoad event listener', () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/units/features.spec.js` around lines 51 - 189, The tests call window.addEventListener across many it blocks (e.g., when invoking provides.tawkOnLoad, tawkOnStatusChange, tawkOnChatMaximized, etc.) but never reset mock state, risking cross-test pollution; add a beforeEach that clears or resets the mock (for example call jest.clearAllMocks() or window.addEventListener.mockClear()/mockReset()) so each listener spec starts with a fresh call history and the expect(window.addEventListener).toHaveBeenCalledWith(...) assertions remain isolated.tsconfig.json (1)
8-9:noImplicitAny: falseis redundant withstrict: false.
strict: falsealready disables all strict-mode checks, includingnoImplicitAny. The explicitnoImplicitAny: falseon line 9 is a no-op and can be removed to reduce noise.♻️ Suggested cleanup
"strict": false, - "noImplicitAny": false, "skipLibCheck": true,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tsconfig.json` around lines 8 - 9, The tsconfig currently sets "strict": false and also redundantly sets "noImplicitAny": false; remove the redundant "noImplicitAny" entry from tsconfig.json so only "strict" controls strictness (leave "strict": false as-is), ensuring the JSON remains valid after deleting the "noImplicitAny" property; reference keys: "strict" and "noImplicitAny".eslint.config.js (1)
6-23:ecmaVersion/sourceTypemisplaced insidelanguageOptions.parserOptionsfor the TypeScript block.In ESLint's flat config format,
ecmaVersionwas moved out ofparserOptionsdirectly intolanguageOptions, as it now controls both syntax and global variable recognition based on the specified ECMAScript version. Nesting them insidelanguageOptions.parserOptionsonly passes them to the parser plugin, not to ESLint's own globals/scope engine. Note the inconsistency: the JS block (lines 28–30) places them correctly at thelanguageOptionslevel.♻️ Suggested fix
{ files: ['**/*.ts'], languageOptions: { parser: tsParser, - parserOptions: { - ecmaVersion: 2020, - sourceType: 'module' - } + ecmaVersion: 2020, + sourceType: 'module' }, plugins: { '@typescript-eslint': tsPlugin },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eslint.config.js` around lines 6 - 23, The TypeScript block currently places ecmaVersion and sourceType under languageOptions.parserOptions; move ecmaVersion and sourceType up one level into languageOptions (alongside parser) so ESLint's globals/scope see them, leaving parserOptions only for parser-specific options for the TypeScript config that references tsParser and tsPlugin; update the object that contains files:['**/*.ts'] and languageOptions to mirror the JS block by placing ecmaVersion and sourceType directly on languageOptions.src/utils/helper.ts (1)
1-7: Redundantvalue.length === 0check — dead code.Because
!valueis evaluated first (short-circuit), an empty string is already caught by the!valuebranch ('' == false). If execution reaches thevalue.length === 0sub-expression,valueis already a truthy string and can never have length 0.♻️ Suggested simplification
function isValidString(value: unknown): boolean { - if (!value || typeof value !== 'string' || value.length === 0) { - return false; - } - - return true; + return typeof value === 'string' && value.length > 0; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/helper.ts` around lines 1 - 7, The conditional in isValidString currently uses !value which conflates non-string falsy values and makes the value.length === 0 check redundant; change the guard to explicitly check type and emptiness by replacing the condition with a strict typeof check plus length check (i.e., ensure typeof value !== 'string' || value.length === 0) so only non-strings or empty strings return false, and then return true otherwise.src/index.ts (2)
11-11:{} as TawkPluginOptionsmasks the missing required fields.The default
options = {} as TawkPluginOptionsuses a type assertion on an empty object that doesn't satisfy the interface (missingpropertyIdandwidgetId). It works at runtime because theisValidStringguards catch it, but a safer alternative is to type the parameter asPartial<TawkPluginOptions>or simply not provide a default (callers should always pass options for a plugin that requires them).♻️ Suggested alternative
- install(app: App, options: TawkPluginOptions = {} as TawkPluginOptions) { + install(app: App, options: TawkPluginOptions) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/index.ts` at line 11, The install method currently uses a forced cast default (options: TawkPluginOptions = {} as TawkPluginOptions) which masks missing required fields; change the parameter to accept Partial<TawkPluginOptions> (e.g., options: Partial<TawkPluginOptions> = {}) or remove the default so callers must supply a full TawkPluginOptions, and update any internal usage in install to continue using the existing isValidString guards (or add explicit runtime validation) to handle absent fields; refer to the install function and the TawkPluginOptions type when making this change.
10-24: Duplicate validation withTawkMessengerconstructor.The
propertyIdandwidgetIdvalidations on lines 12–20 are duplicated insrc/lib/index.ts(lines 26–32). Consider removing one site — either validate only here in the plugin entry point (and trust the constructor), or only in the constructor (and remove validation frominstall). Having both is not harmful but adds unnecessary duplication.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/index.ts` around lines 10 - 24, The install method on the TawkMessengerVue plugin duplicates propertyId/widgetId checks already performed in the TawkMessenger constructor; remove the redundant validation in TawkMessengerVue.install (the isValidString checks and their console.error returns) and simply invoke new TawkMessenger(app, options) so the constructor (class TawkMessenger in src/lib/index.ts) is the single place that validates and errors; ensure you keep the existing error handling/log messages in the TawkMessenger constructor so validation feedback remains intact.src/lib/index.ts (2)
25-43: Earlyreturnin constructor leavesthis.appuninitialized.If
isValidStringfails, the constructor returns before assigningthis.app,this.propertyId, etc. While currently harmless (the instance isn't stored), TypeScript considers the object fully constructed. If future code ever accesses a property on the returned instance, it will beundefineddespite the type claimingApp.Consider either throwing an error on invalid input or using the definite assignment assertion pattern to make this explicit.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/index.ts` around lines 25 - 43, The constructor currently returns early on invalid options (calls to isValidString for propertyId/widgetId), leaving instance fields like this.app, this.propertyId, this.widgetId uninitialized; change this behavior by validating inputs and throwing a descriptive Error when validation fails (e.g., inside the constructor before any assignments) or explicitly mark fields with definite assignment assertions and document expected usage; update the constructor in the class containing constructor(app: App, options: TawkPluginOptions) and ensure any call sites handle the thrown error, keeping the call to this.load() only after successful validation and assignment.
181-301: Event listeners accumulate with no cleanup mechanism.Each call to a provided listener (e.g.,
tawkOnLoad(cb)) adds a newwindow.addEventListenerwith an anonymous wrapper. There's no way for consumers to remove these listeners, and calling the provider multiple times (e.g., across component mounts) stacks duplicate handlers. Consider returning a cleanup/unsubscribe function so consumers can manage listener lifecycle, or deduplicate internally.♻️ Example: return a cleanup function
this.app.provide('tawkOnLoad', (callback: TawkCallback) => { - window.addEventListener('tawkLoad', () => { - callback(); - }); + const handler = () => callback(); + window.addEventListener('tawkLoad', handler); + return () => window.removeEventListener('tawkLoad', handler); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/index.ts` around lines 181 - 301, The provided event listener providers (e.g., tawkOnLoad, tawkOnStatusChange, tawkOnBeforeLoad, tawkOnChatMaximized, tawkOnChatMinimized, tawkOnChatHidden, tawkOnChatStarted, tawkOnChatEnded, tawkOnPrechatSubmit, tawkOnOfflineSubmit, tawkOnChatMessageVisitor, tawkOnChatMessageAgent, tawkOnChatMessageSystem, tawkOnAgentJoinChat, tawkOnAgentLeaveChat, tawkOnChatSatisfaction, tawkOnVisitorNameChanged, tawkOnFileUpload, tawkOnTagsUpdated, tawkOnUnreadCountChanged) currently add anonymous window.addEventListener handlers with no way to remove them; change each provider to create a named/closed-over handler, call window.addEventListener with that handler, and return an unsubscribe function that calls window.removeEventListener with the same handler so callers can clean up on unmount; alternatively implement internal deduplication by storing handlers per event name and returning a remover that unregisters that specific callback.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@package.json`:
- Around line 49-51: Move "vue": "^3.5.28" out of "dependencies" and add it
under "peerDependencies" in package.json (so consumers provide Vue), and
optionally add it under "devDependencies" for local development/testing if
needed; update the package.json entries to remove "vue" from the "dependencies"
object and create/augment the "peerDependencies" object with "vue": "^3.5.28" so
tooling and consumers recognize the peer requirement.
In `@src/lib/index.ts`:
- Around line 311-316: The provider functions (e.g., tawkSetAttributes) declare
the callback parameter as required but the TawkAPI methods accept an optional
callback; change the provider signatures to make the callback optional (e.g.,
callback?: TawkErrorCallback) so callers can omit it, and forward that optional
callback to window.Tawk_API.setAttributes(attribute, callback) unchanged; apply
the same change to the other provider methods mentioned (tawkAddAttributes,
tawkGetAttributes, tawkRemoveAttributes, tawkOnPrechatSubmit or whichever
provider names appear in the diff) so all callbacks match the TawkAPI optional
signature.
- Around line 91-131: The provided action/getter functions (e.g., in
provideActions: tawkStart, tawkShutdown, tawkMaximize and the getters like
tawkGetStatus) call window.Tawk_API methods directly and can throw if the widget
hasn't loaded; add a small runtime guard/queue helper (e.g., create a local
callTawk(functionName, ...args) used by provideActions/getters) that: 1) checks
window.Tawk_API exists and typeof window.Tawk_API[functionName] === 'function'
and calls it immediately if present; 2) otherwise pushes the call into a simple
queue (e.g., window.__tawkQueue) to be flushed when the Tawk script signals
ready (or when window.Tawk_API becomes available); replace direct calls in
tawkStart/tawkShutdown/tawkMaximize/tawkMinimize/tawkToggle/tawkPopup/tawkShowWidget/tawkHideWidget/tawkToggleVisibility/tawkEndChat
and the getters to use this helper.
- Around line 45-48: The SSR guard in the load() method incorrectly checks
undeclared globals using "!window" which throws ReferenceError during
server-side rendering; update the guard in load() to use typeof checks (e.g.,
typeof window === 'undefined' || typeof document === 'undefined') so it safely
returns on the server; locate the load() method in src/lib/index.ts and replace
the current condition with the typeof-based checks to prevent accessing
undefined globals.
In `@src/types/index.ts`:
- Around line 63-112: The TawkAPI interface currently declares all action/getter
methods as required while window.Tawk_API is initialized to {} (so calls like
tawkMaximize or other provider wrappers can hit TypeError before the widget
loads); update the TawkAPI interface to make runtime methods optional (e.g.,
start?(): void; maximize?(): void; getStatus?(): 'online'|'away'|'offline';
etc.) and add runtime guards in the provider wrappers (where you call
window.Tawk_API, e.g., tawkMaximize/tawkMinimize/tawkGetStatus) to check the
method exists before invoking (or return/throw a clear error instructing
consumers to wait for tawkOnLoad), ensuring no direct unguarded calls to
window.Tawk_API.<method>() occur.
In `@vite.config.ts`:
- Line 9: The UMD/global name in the Vite build config (build.lib.name) uses
hyphens ('tawk-messenger-vue-3'), which is not a valid JS identifier; change
build.lib.name in vite.config.ts to a valid identifier (e.g. tawkMessengerVue3
or TawkMessengerVue3) so the generated UMD global is accessible as
window.tawkMessengerVue3; keep the value as an identifier (no spaces or
hyphens), update any tests or consumers that expect the old string, and ensure
build.lib.name is the only place changed.
---
Outside diff comments:
In `@package.json`:
- Around line 19-23: The package.json contains a stale nested "repository" field
with the value "tawk/tawk-messenger-vue-2"; update or remove that nested key so
the repository metadata is correct (e.g., remove the nested "repository"
property or change its value to "tawk/tawk-messenger-vue-3") by editing the
"repository" object in package.json where the keys "type", "url", and the
incorrect "repository" currently appear.
In `@src/utils/widget.ts`:
- Around line 40-41: The current insertion uses
getElementsByTagName('script')[0] (assigned to firstScript) and then calls
firstScript.parentNode.insertBefore(script, firstScript), which can crash if
firstScript is undefined or parentNode is null; change the logic in the widget
initialization to check whether firstScript exists and has a parentNode and, if
not, fall back to a safe container (e.g., document.head,
document.getElementsByTagName('head')[0], or document.body) and use
appendChild/insertBefore on that container instead of assuming
firstScript.parentNode is present; update the code paths that reference
firstScript and insertBefore(script, firstScript) so they use the guarded
container to avoid runtime errors in environments without script tags.
---
Nitpick comments:
In `@eslint.config.js`:
- Around line 6-23: The TypeScript block currently places ecmaVersion and
sourceType under languageOptions.parserOptions; move ecmaVersion and sourceType
up one level into languageOptions (alongside parser) so ESLint's globals/scope
see them, leaving parserOptions only for parser-specific options for the
TypeScript config that references tsParser and tsPlugin; update the object that
contains files:['**/*.ts'] and languageOptions to mirror the JS block by placing
ecmaVersion and sourceType directly on languageOptions.
In `@src/index.ts`:
- Line 11: The install method currently uses a forced cast default (options:
TawkPluginOptions = {} as TawkPluginOptions) which masks missing required
fields; change the parameter to accept Partial<TawkPluginOptions> (e.g.,
options: Partial<TawkPluginOptions> = {}) or remove the default so callers must
supply a full TawkPluginOptions, and update any internal usage in install to
continue using the existing isValidString guards (or add explicit runtime
validation) to handle absent fields; refer to the install function and the
TawkPluginOptions type when making this change.
- Around line 10-24: The install method on the TawkMessengerVue plugin
duplicates propertyId/widgetId checks already performed in the TawkMessenger
constructor; remove the redundant validation in TawkMessengerVue.install (the
isValidString checks and their console.error returns) and simply invoke new
TawkMessenger(app, options) so the constructor (class TawkMessenger in
src/lib/index.ts) is the single place that validates and errors; ensure you keep
the existing error handling/log messages in the TawkMessenger constructor so
validation feedback remains intact.
In `@src/lib/index.ts`:
- Around line 25-43: The constructor currently returns early on invalid options
(calls to isValidString for propertyId/widgetId), leaving instance fields like
this.app, this.propertyId, this.widgetId uninitialized; change this behavior by
validating inputs and throwing a descriptive Error when validation fails (e.g.,
inside the constructor before any assignments) or explicitly mark fields with
definite assignment assertions and document expected usage; update the
constructor in the class containing constructor(app: App, options:
TawkPluginOptions) and ensure any call sites handle the thrown error, keeping
the call to this.load() only after successful validation and assignment.
- Around line 181-301: The provided event listener providers (e.g., tawkOnLoad,
tawkOnStatusChange, tawkOnBeforeLoad, tawkOnChatMaximized, tawkOnChatMinimized,
tawkOnChatHidden, tawkOnChatStarted, tawkOnChatEnded, tawkOnPrechatSubmit,
tawkOnOfflineSubmit, tawkOnChatMessageVisitor, tawkOnChatMessageAgent,
tawkOnChatMessageSystem, tawkOnAgentJoinChat, tawkOnAgentLeaveChat,
tawkOnChatSatisfaction, tawkOnVisitorNameChanged, tawkOnFileUpload,
tawkOnTagsUpdated, tawkOnUnreadCountChanged) currently add anonymous
window.addEventListener handlers with no way to remove them; change each
provider to create a named/closed-over handler, call window.addEventListener
with that handler, and return an unsubscribe function that calls
window.removeEventListener with the same handler so callers can clean up on
unmount; alternatively implement internal deduplication by storing handlers per
event name and returning a remover that unregisters that specific callback.
In `@src/utils/helper.ts`:
- Around line 1-7: The conditional in isValidString currently uses !value which
conflates non-string falsy values and makes the value.length === 0 check
redundant; change the guard to explicitly check type and emptiness by replacing
the condition with a strict typeof check plus length check (i.e., ensure typeof
value !== 'string' || value.length === 0) so only non-strings or empty strings
return false, and then return true otherwise.
In `@tests/units/features.spec.js`:
- Around line 51-189: The tests call window.addEventListener across many it
blocks (e.g., when invoking provides.tawkOnLoad, tawkOnStatusChange,
tawkOnChatMaximized, etc.) but never reset mock state, risking cross-test
pollution; add a beforeEach that clears or resets the mock (for example call
jest.clearAllMocks() or window.addEventListener.mockClear()/mockReset()) so each
listener spec starts with a fresh call history and the
expect(window.addEventListener).toHaveBeenCalledWith(...) assertions remain
isolated.
In `@tsconfig.json`:
- Around line 8-9: The tsconfig currently sets "strict": false and also
redundantly sets "noImplicitAny": false; remove the redundant "noImplicitAny"
entry from tsconfig.json so only "strict" controls strictness (leave "strict":
false as-is), ensuring the JSON remains valid after deleting the "noImplicitAny"
property; reference keys: "strict" and "noImplicitAny".
| "dependencies": { | ||
| "vue": "^3.2.31" | ||
| "vue": "^3.5.28" | ||
| }, |
There was a problem hiding this comment.
vue should be a peerDependency, not a dependency.
For a Vue plugin library, listing vue under dependencies causes consumers to potentially install a duplicate Vue instance, which leads to runtime errors (multiple Vue instances, broken inject/provide, etc.). Move it to peerDependencies instead.
♻️ Suggested fix
- "dependencies": {
- "vue": "^3.5.28"
- },
+ "peerDependencies": {
+ "vue": "^3.5.0"
+ },📝 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.
| "dependencies": { | |
| "vue": "^3.2.31" | |
| "vue": "^3.5.28" | |
| }, | |
| "peerDependencies": { | |
| "vue": "^3.5.0" | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` around lines 49 - 51, Move "vue": "^3.5.28" out of
"dependencies" and add it under "peerDependencies" in package.json (so consumers
provide Vue), and optionally add it under "devDependencies" for local
development/testing if needed; update the package.json entries to remove "vue"
from the "dependencies" object and create/augment the "peerDependencies" object
with "vue": "^3.5.28" so tooling and consumers recognize the peer requirement.
| load(): void { | ||
| if (!window || !document) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
SSR guard !window will throw ReferenceError instead of returning.
In server-side rendering (e.g., Nuxt SSR), window is not defined at all. Accessing an undeclared global with !window throws a ReferenceError rather than evaluating to true. Use typeof window === 'undefined' instead.
🐛 Proposed fix
load(): void {
- if (!window || !document) {
+ if (typeof window === 'undefined' || typeof document === 'undefined') {
return;
}📝 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.
| load(): void { | |
| if (!window || !document) { | |
| return; | |
| } | |
| load(): void { | |
| if (typeof window === 'undefined' || typeof document === 'undefined') { | |
| return; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/index.ts` around lines 45 - 48, The SSR guard in the load() method
incorrectly checks undeclared globals using "!window" which throws
ReferenceError during server-side rendering; update the guard in load() to use
typeof checks (e.g., typeof window === 'undefined' || typeof document ===
'undefined') so it safely returns on the server; locate the load() method in
src/lib/index.ts and replace the current condition with the typeof-based checks
to prevent accessing undefined globals.
| provideActions(): void { | ||
| this.app.provide('tawkStart', () => { | ||
| window.Tawk_API.start(); | ||
| }); | ||
|
|
||
| this.app.provide('tawkShutdown', () => { | ||
| window.Tawk_API.shutdown(); | ||
| }); | ||
|
|
||
| this.app.provide('tawkMaximize', () => { | ||
| window.Tawk_API.maximize(); | ||
| }); | ||
|
|
||
| this.app.provide('tawkMinimize', () => { | ||
| window.Tawk_API.minimize(); | ||
| }); | ||
|
|
||
| this.app.provide('tawkToggle', () => { | ||
| window.Tawk_API.toggle(); | ||
| }); | ||
|
|
||
| this.app.provide('tawkPopup', () => { | ||
| window.Tawk_API.popup(); | ||
| }); | ||
|
|
||
| this.app.provide('tawkShowWidget', () => { | ||
| window.Tawk_API.showWidget(); | ||
| }); | ||
|
|
||
| this.app.provide('tawkHideWidget', () => { | ||
| window.Tawk_API.hideWidget(); | ||
| }); | ||
|
|
||
| this.app.provide('tawkToggleVisibility', () => { | ||
| window.Tawk_API.toggleVisibility(); | ||
| }); | ||
|
|
||
| this.app.provide('tawkEndChat', () => { | ||
| window.Tawk_API.endChat(); | ||
| }); | ||
| } |
There was a problem hiding this comment.
No guard against calling Tawk_API methods before the widget has loaded.
All provided action and getter functions (e.g., tawkStart, tawkGetStatus) directly invoke window.Tawk_API.<method>(). Since the Tawk script loads asynchronously, these methods don't exist on the initially empty {} object. Calling them before the widget is ready will throw TypeError.
Consider adding a runtime guard or queueing calls until the widget is loaded.
Also applies to: 136-176
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/index.ts` around lines 91 - 131, The provided action/getter functions
(e.g., in provideActions: tawkStart, tawkShutdown, tawkMaximize and the getters
like tawkGetStatus) call window.Tawk_API methods directly and can throw if the
widget hasn't loaded; add a small runtime guard/queue helper (e.g., create a
local callTawk(functionName, ...args) used by provideActions/getters) that: 1)
checks window.Tawk_API exists and typeof window.Tawk_API[functionName] ===
'function' and calls it immediately if present; 2) otherwise pushes the call
into a simple queue (e.g., window.__tawkQueue) to be flushed when the Tawk
script signals ready (or when window.Tawk_API becomes available); replace direct
calls in
tawkStart/tawkShutdown/tawkMaximize/tawkMinimize/tawkToggle/tawkPopup/tawkShowWidget/tawkHideWidget/tawkToggleVisibility/tawkEndChat
and the getters to use this helper.
| this.app.provide('tawkSetAttributes', ( | ||
| attribute: Record<string, unknown>, | ||
| callback: TawkErrorCallback | ||
| ) => { | ||
| window.Tawk_API.setAttributes(attribute, callback); | ||
| }); |
There was a problem hiding this comment.
Setter callbacks are required here but optional in the TawkAPI interface.
The provided setter functions require callback as a mandatory parameter (e.g., line 313: callback: TawkErrorCallback), but the corresponding TawkAPI methods declare it as optional (callback?: TawkErrorCallback). This forces consumers to always pass a callback even when they don't need one.
♻️ Suggested fix (make callbacks optional)
this.app.provide('tawkSetAttributes', (
attribute: Record<string, unknown>,
- callback: TawkErrorCallback
+ callback?: TawkErrorCallback
) => {
window.Tawk_API.setAttributes(attribute, callback);
});
this.app.provide('tawkAddEvent', (
event: string,
- metadata: Record<string, unknown>,
- callback: TawkErrorCallback
+ metadata?: Record<string, unknown>,
+ callback?: TawkErrorCallback
) => {
window.Tawk_API.addEvent(event, metadata, callback);
});
- this.app.provide('tawkAddTags', (tags: string[], callback: TawkErrorCallback) => {
+ this.app.provide('tawkAddTags', (tags: string[], callback?: TawkErrorCallback) => {
window.Tawk_API.addTags(tags, callback);
});
- this.app.provide('tawkRemoveTags', (tags: string[], callback: TawkErrorCallback) => {
+ this.app.provide('tawkRemoveTags', (tags: string[], callback?: TawkErrorCallback) => {
window.Tawk_API.removeTags(tags, callback);
});
- this.app.provide('tawkSwitchWidget', (data: TawkSwitchWidgetData, callback: TawkCallback) => {
+ this.app.provide('tawkSwitchWidget', (data: TawkSwitchWidgetData, callback?: TawkCallback) => {
window.Tawk_API.switchWidget(data, callback);
});Also applies to: 318-324, 326-328, 330-332, 334-336
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/index.ts` around lines 311 - 316, The provider functions (e.g.,
tawkSetAttributes) declare the callback parameter as required but the TawkAPI
methods accept an optional callback; change the provider signatures to make the
callback optional (e.g., callback?: TawkErrorCallback) so callers can omit it,
and forward that optional callback to window.Tawk_API.setAttributes(attribute,
callback) unchanged; apply the same change to the other provider methods
mentioned (tawkAddAttributes, tawkGetAttributes, tawkRemoveAttributes,
tawkOnPrechatSubmit or whichever provider names appear in the diff) so all
callbacks match the TawkAPI optional signature.
| export interface TawkAPI { | ||
| // Mutable properties set before widget loads | ||
| visitor?: TawkVisitorData; | ||
| customStyle?: TawkCustomStyle; | ||
| autoStart?: boolean; | ||
| embedded?: string; | ||
|
|
||
| // State flags (read-only, set by the widget) | ||
| onLoaded?: boolean; | ||
| onBeforeLoaded?: boolean; | ||
|
|
||
| // --- Actions --- | ||
| start(): void; | ||
| shutdown(): void; | ||
| maximize(): void; | ||
| minimize(): void; | ||
| toggle(): void; | ||
| popup(): void; | ||
| showWidget(): void; | ||
| hideWidget(): void; | ||
| toggleVisibility(): void; | ||
| endChat(): void; | ||
|
|
||
| // --- Getters --- | ||
| getWindowType(): 'inline' | 'embed'; | ||
| getStatus(): 'online' | 'away' | 'offline'; | ||
| isChatMaximized(): boolean; | ||
| isChatMinimized(): boolean; | ||
| isChatHidden(): boolean; | ||
| isChatOngoing(): boolean; | ||
| isVisitorEngaged(): boolean; | ||
| widgetPosition(): string; | ||
|
|
||
| // --- Setters --- | ||
| setAttributes( | ||
| attributes: Record<string, unknown>, | ||
| callback?: TawkErrorCallback | ||
| ): void; | ||
|
|
||
| addEvent( | ||
| name: string, | ||
| metadata?: Record<string, unknown>, | ||
| callback?: TawkErrorCallback | ||
| ): void; | ||
|
|
||
| addTags(tags: string[], callback?: TawkErrorCallback): void; | ||
|
|
||
| removeTags(tags: string[], callback?: TawkErrorCallback): void; | ||
|
|
||
| switchWidget(data: TawkSwitchWidgetData, callback?: TawkCallback): void; |
There was a problem hiding this comment.
All TawkAPI methods are declared as required, but the object is initialized as {}.
In src/lib/index.ts line 53, window.Tawk_API is initialized as (window.Tawk_API || {}) as TawkAPI. Since the Tawk widget script loads asynchronously, none of these methods exist on the object until the script finishes loading. Consumers calling provided functions (e.g., tawkMaximize) before the widget is ready will hit a TypeError: window.Tawk_API.maximize is not a function.
Consider either:
- Making the action/getter methods optional in the interface (e.g.,
start?(): void;) and adding runtime guards in the provider wrappers, or - Documenting that consumers must wait for the
tawkOnLoadevent before calling actions/getters.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/types/index.ts` around lines 63 - 112, The TawkAPI interface currently
declares all action/getter methods as required while window.Tawk_API is
initialized to {} (so calls like tawkMaximize or other provider wrappers can hit
TypeError before the widget loads); update the TawkAPI interface to make runtime
methods optional (e.g., start?(): void; maximize?(): void; getStatus?():
'online'|'away'|'offline'; etc.) and add runtime guards in the provider wrappers
(where you call window.Tawk_API, e.g., tawkMaximize/tawkMinimize/tawkGetStatus)
to check the method exists before invoking (or return/throw a clear error
instructing consumers to wait for tawkOnLoad), ensuring no direct unguarded
calls to window.Tawk_API.<method>() occur.
| build: { | ||
| lib: { | ||
| entry: fileURLToPath(new URL('src/index.ts', import.meta.url)), | ||
| name: 'tawk-messenger-vue-3', |
There was a problem hiding this comment.
UMD global name contains hyphens — invalid JavaScript identifier.
The name field in a Vite library build is used verbatim as the UMD global variable name. 'tawk-messenger-vue-3' is not a valid JavaScript identifier (hyphens are the subtraction operator), so the UMD bundle would be inaccessible as window.tawk-messenger-vue-3. UMD consumers would need the awkward bracket form window['tawk-messenger-vue-3'], and some bundlers may silently produce a broken UMD wrapper.
Use a valid identifier for the global:
🐛 Proposed fix
- name: 'tawk-messenger-vue-3',
+ name: 'TawkMessengerVue3',📝 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.
| name: 'tawk-messenger-vue-3', | |
| name: 'TawkMessengerVue3', |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@vite.config.ts` at line 9, The UMD/global name in the Vite build config
(build.lib.name) uses hyphens ('tawk-messenger-vue-3'), which is not a valid JS
identifier; change build.lib.name in vite.config.ts to a valid identifier (e.g.
tawkMessengerVue3 or TawkMessengerVue3) so the generated UMD global is
accessible as window.tawkMessengerVue3; keep the value as an identifier (no
spaces or hyphens), update any tests or consumers that expect the old string,
and ensure build.lib.name is the only place changed.
2 participants
Summary by CodeRabbit