feat: v0.2.0 inbound/outbound architecture + 5 new features#13
feat: v0.2.0 inbound/outbound architecture + 5 new features#13jackmisner merged 4 commits intomainfrom
Conversation
…rms, link decoration, and event callbacks Restructure src/core/ into src/inbound/, src/outbound/, src/common/ directories. Add 5 new features: event lifecycle callbacks, first-touch/last-touch attribution, UTM link builder with validation, form field population (vanilla JS + React), and automatic link decoration (vanilla JS + React). Update README with full documentation for all new features. 🤖 Generated with [Nori](https://nori.ai) 🤖 Generated with [Nori](https://nori.ai) Co-Authored-By: Nori <contact@tilework.tech>
WalkthroughReorganises exports into inbound/outbound/common, adds multi-mode attribution (first/last/both) with configurable suffixes, introduces lifecycle callbacks (onCapture, onStore, onClear, onAppend, onExpire), expands React surface (hidden fields, link decorator, hooks), updates storage API (ClearOptions overload) and broadens public types and outbound builder/decorator features. Changes
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
Comment |
…nt duplicate hidden inputs and observer loops - Hook now calls storeWithAttribution() instead of storeUtmParameters() so attribution mode (first/last/both) actually works through useUtmTracking - Forward onCapture, onStore, onClear, onAppend callbacks from config to the underlying core functions - Wrap firstTouchParams/lastTouchParams in useMemo to avoid re-reading storage on every render - autoCreateFields now checks for existing hidden inputs before creating duplicates - observeAndDecorateLinks uses a re-entrancy guard to prevent infinite loops caused by its own href mutations 🤖 Generated with [Nori](https://nori.ai) 🤖 Generated with [Nori](https://nori.ai) Co-Authored-By: Nori <contact@tilework.tech>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
__tests__/common/validator.test.ts (1)
1-12:⚠️ Potential issue | 🟠 MajorRestore
defaultProtocolafter each test.
setDefaultProtocol()mutates module state; without restoration, later suites can inherit the modified value and behave differently depending on run order.Based on learnings: In `validator.ts`, manage module-level mutable state `defaultProtocol` carefully — tests that call `setDefaultProtocol()` must restore the original value.Proposed fix
-import { describe, it, expect, beforeEach } from 'vitest' +import { describe, it, expect, beforeEach, afterEach } from 'vitest' @@ describe('getDefaultProtocol / setDefaultProtocol', () => { + let originalProtocol: string + beforeEach(() => { + originalProtocol = getDefaultProtocol() setDefaultProtocol('https://') }) + + afterEach(() => { + setDefaultProtocol(originalProtocol) + })Also applies to: 171-192
🤖 Fix all issues with AI agents
In `@src/inbound/form.ts`:
- Around line 140-154: The autoCreateFields function currently appends hidden
inputs blindly which can create duplicates; update it to, for each form (forms)
and each param key in UtmParameters, first check the form for existing inputs
with the same name (e.g., query the form for input[name=key] or
input[name="${key}"]) and if one exists update its value instead of appending a
new element, otherwise create and append the hidden input; ensure the increment
of count only happens when a new input is created (not when updating an existing
input) and keep references to the function name autoCreateFields, the forms
variable, and the input creation branch to locate the change.
- Around line 11-25: FormPopulateOptions declares an unused touch property
causing confusion; either remove touch from the interface or wire it into the
form population logic. Locate the FormPopulateOptions interface and either
delete the touch?: TouchType declaration, or update populateFormFields and
createUtmHiddenFields to accept/use options.touch when selecting which Touch
(e.g., first/last) to read from storageKey/storageType; ensure any references to
TouchType are updated and unit tests adjusted accordingly.
In `@src/outbound/decorator.ts`:
- Around line 116-131: The function observeAndDecorateLinks can throw if
document.body is not present; before calling observer.observe(document.body,
...) guard with a null check (if (!document.body) return () => {}) so the
function becomes a no-op when run in <head> or before DOM ready, and ensure you
return a cleanup function that calls observer.disconnect() when observation is
started; reference observeAndDecorateLinks, decorateLinks, MutationObserver,
observer.observe and observer.disconnect to locate the changes.
- Around line 12-58: The public option touch in LinkDecoratorOptions is declared
but unused; update decorateLinks to destructure touch from options and pass it
into getStoredUtmParameters so callers can select first/last touch, or remove
the option entirely; specifically, add touch?: TouchType to the destructuring in
decorateLinks (alongside storageKey/storageType) and call
getStoredUtmParameters({ storageKey, storageType, touch }) (or update that
helper's signature to accept a touch arg), and ensure any types (TouchType and
getStoredUtmParameters param type) are updated accordingly.
🧹 Nitpick comments (13)
src/outbound/builder.ts (1)
12-12: Consider documenting the UNSAFE_CHARS constant.The regex
/[&=?#]/captures characters that would interfere with URL parsing. A brief inline comment explaining why these specific characters are unsafe for UTM values would improve maintainability.📝 Proposed documentation
-const UNSAFE_CHARS = /[&=?#]/ +/** Characters that break URL query string parsing when used in UTM values */ +const UNSAFE_CHARS = /[&=?#]/__tests__/outbound/appender.test.ts (1)
1-6: Import path correctly updated; consider adding tests foronAppendcallback.The import path update aligns with the module restructure. However, the new
onAppendcallback feature inappendUtmParameterslacks test coverage in this file. Consider adding tests to verify:
- Callback is invoked with correct URL and params
- Callback errors don't break the append operation
🧪 Suggested test cases
describe('onAppend callback', () => { it('invokes onAppend with result URL and params', () => { const onAppend = vi.fn() appendUtmParameters( 'https://example.com', { utm_source: 'test' }, { onAppend }, ) expect(onAppend).toHaveBeenCalledWith( 'https://example.com/?utm_source=test', { utm_source: 'test' }, ) }) it('does not throw when onAppend throws', () => { const onAppend = vi.fn(() => { throw new Error('callback error') }) expect(() => appendUtmParameters('https://example.com', { utm_source: 'test' }, { onAppend }), ).not.toThrow() }) })__tests__/react/UtmLinkDecorator.test.tsx (1)
42-47: Consider removing unnecessarystoreUtmParameterscall and expanding hook test.Line 43 stores UTM parameters, but this test only verifies the ref object structure—the stored params aren't used. Additionally, the hook test could be more comprehensive by verifying actual decoration behaviour when the ref is attached to a container.
🧪 Suggested improvements
describe('useUtmLinkDecorator', () => { beforeEach(() => { sessionStorage.clear() document.body.replaceChildren() }) it('returns a ref object', () => { - storeUtmParameters({ utm_source: 'google' }) const { result } = renderHook(() => useUtmLinkDecorator()) expect(result.current).toBeDefined() expect(result.current).toHaveProperty('current') }) + + it('decorates links when ref is attached to container with stored params', () => { + storeUtmParameters({ utm_source: 'test' }) + function TestComponent() { + const ref = useUtmLinkDecorator() + return ( + <div ref={ref}> + <a href="https://example.com">Link</a> + </div> + ) + } + const { container } = render(<TestComponent />) + const link = container.querySelector('a') as HTMLAnchorElement + expect(link.href).toContain('utm_source=test') + }) })src/react/UtmHiddenFields.tsx (1)
12-21: Unusedtouchprop in interface and component.The
touchprop is declared inUtmHiddenFieldsPropsbut is never destructured or used in the component logic. This may be an incomplete feature or dead code.♻️ Either remove the unused prop or implement the intended behaviour
If
touchis not needed:export interface UtmHiddenFieldsProps { - touch?: TouchType keyFormat?: KeyFormat prefix?: string storageKey?: string storageType?: 'session' | 'local' }If
touchshould influence which storage key is read (e.g., for first-touch vs last-touch attribution), consider passing it togetStoredUtmParametersor deriving the storage key from it.src/react/useUtmFormData.ts (1)
11-19: Unusedtouchoption in interface and hook.Similar to
UtmHiddenFields, thetouchoption is declared inUseUtmFormDataOptionsbut never used in the hook logic. This creates a misleading API where consumers might expect the option to affect behaviour.♻️ Either remove the unused option or implement the intended behaviour
If
touchis not needed:export interface UseUtmFormDataOptions { - touch?: TouchType storageKey?: string storageType?: 'session' | 'local' keyFormat?: 'snake_case' | 'camelCase' }If
touchshould select first-touch vs last-touch data, you would need to derive the storage key or pass it through to the storage call.__tests__/outbound/decorator.test.ts (1)
131-151: LGTM with optional enhancement suggestion.The test suite provides good coverage for
decorateLinksincluding host filtering, skipExisting behaviour, and callbacks. TheobserveAndDecorateLinkstests verify the essential contract (cleanup function and immediate decoration).💡 Consider adding a test for MutationObserver behaviour
The current tests don't verify that dynamically added links are decorated after the initial call. This would require triggering the MutationObserver:
it('decorates links added after initial call', async () => { storeUtmParameters({ utm_source: 'google' }) const cleanup = observeAndDecorateLinks() // Add link after observer is set up const newLink = createLink('https://example.com/new-page') // MutationObserver is async, wait for it await new Promise(resolve => setTimeout(resolve, 0)) expect(newLink.href).toContain('utm_source=google') cleanup() })This is optional since verifying MutationObserver behaviour in jsdom can be brittle.
src/react/useUtmTracking.ts (1)
235-254: Consider memoising attribution parameter retrieval to avoid redundant storage reads.The
firstTouchParamsandlastTouchParamsvalues are computed synchronously during every render by callinggetStoredUtmParameters. Since these depend only on stable config values (config.storageKey,config.attribution,config.keyFormat,config.storageType), they will produce identical results across re-renders unless storage is externally modified.For components that re-render frequently, this could result in unnecessary
JSON.parsecalls on each render.💡 Proposed optimisation using useMemo
+ // Attribution mode determines which touch params are available + const attributionMode = config.attribution?.mode ?? 'last' + + const { firstTouchParams, lastTouchParams } = useMemo(() => { + const first = + attributionMode === 'last' + ? null + : getStoredUtmParameters({ + storageKey: config.storageKey + (config.attribution?.firstTouchSuffix ?? '_first'), + keyFormat: config.keyFormat, + storageType: config.storageType, + }) + const last = + attributionMode === 'first' + ? null + : attributionMode === 'both' + ? getStoredUtmParameters({ + storageKey: config.storageKey + (config.attribution?.lastTouchSuffix ?? '_last'), + keyFormat: config.keyFormat, + storageType: config.storageType, + }) + : utmParameters + return { firstTouchParams: first, lastTouchParams: last } + }, [ + attributionMode, + config.storageKey, + config.attribution?.firstTouchSuffix, + config.attribution?.lastTouchSuffix, + config.keyFormat, + config.storageType, + utmParameters, + ]) - // Attribution mode determines which touch params are available - const attributionMode = config.attribution?.mode ?? 'last' - const firstTouchParams = - attributionMode === 'last' - ? null - : getStoredUtmParameters({ - storageKey: config.storageKey + (config.attribution?.firstTouchSuffix ?? '_first'), - keyFormat: config.keyFormat, - storageType: config.storageType, - }) - const lastTouchParams = - attributionMode === 'first' - ? null - : attributionMode === 'both' - ? getStoredUtmParameters({ - storageKey: config.storageKey + (config.attribution?.lastTouchSuffix ?? '_last'), - keyFormat: config.keyFormat, - storageType: config.storageType, - }) - : utmParameterssrc/config/loader.ts (1)
241-384: Consider adding validation forattributionconfig and event callbacks.The
validateConfigfunction validatessanitizeandpiiFilteringnested configs but doesn't validate the newattributionconfig (mode, firstTouchSuffix, lastTouchSuffix) or event callbacks (onCapture, onStore, etc.). For consistency, consider adding validation for:
attribution.modemust be'last' | 'first' | 'both'attribution.firstTouchSuffixandlastTouchSuffixmust be strings if provided- Event callbacks must be functions if provided
♻️ Proposed validation additions
if (c.piiFiltering !== undefined) { // ... existing piiFiltering validation ... } + + if (c.attribution !== undefined) { + if (typeof c.attribution !== 'object' || c.attribution === null || Array.isArray(c.attribution)) { + errors.push('attribution must be an object') + } else { + const a = c.attribution as Record<string, unknown> + if (a.mode !== undefined && a.mode !== 'last' && a.mode !== 'first' && a.mode !== 'both') { + errors.push('attribution.mode must be "last", "first", or "both"') + } + if (a.firstTouchSuffix !== undefined && typeof a.firstTouchSuffix !== 'string') { + errors.push('attribution.firstTouchSuffix must be a string') + } + if (a.lastTouchSuffix !== undefined && typeof a.lastTouchSuffix !== 'string') { + errors.push('attribution.lastTouchSuffix must be a string') + } + } + } + + const callbackFields = ['onCapture', 'onStore', 'onClear', 'onAppend', 'onExpire'] as const + for (const field of callbackFields) { + if (c[field] !== undefined && typeof c[field] !== 'function') { + errors.push(`${field} must be a function`) + } + } return errors }src/react/UtmLinkDecorator.tsx (2)
20-39: Missingoptionsin useEffect dependency array.The
useEffectreferencesoptionsandoptions.selectorbut has an empty dependency array. While the comment indicates decoration runs "on mount", this will cause eslint'sreact-hooks/exhaustive-depsrule to warn. If mount-only behaviour is intentional, consider either:
- Suppressing the lint rule with a comment explaining the intent
- Using
useRefto capture options at mount timeAdditionally, the
linksquery on line 25 is only used as a guard — the actual decoration re-queries viadecorateLinks. This is fine but slightly redundant.♻️ Option 1: Add lint suppression with explanation
useEffect(() => { if (!ref.current) return // Scope decoration to this container const container = ref.current const links = container.querySelectorAll<HTMLAnchorElement>(options.selector ?? 'a[href]') if (links.length === 0) return // Use decorateLinks with a scoped selector is tricky since it queries document. // Instead, manually scope: temporarily add a unique attribute and use it as selector. const scopeId = `__utm_decorator_${Date.now()}` container.setAttribute('data-utm-scope', scopeId) decorateLinks({ ...options, selector: `[data-utm-scope="${scopeId}"] ${options.selector ?? 'a[href]'}`, }) container.removeAttribute('data-utm-scope') + // eslint-disable-next-line react-hooks/exhaustive-deps -- Intentionally run only on mount }, [])
51-56: Redundant type cast on ref.The cast
ref as React.RefObject<HTMLDivElement>is unnecessary sincerefis already typed asReact.RefObject<HTMLDivElement | null>, which is assignable to therefprop expectingReact.RefObject<HTMLDivElement>.♻️ Proposed simplification
export function UtmLinkDecorator(props: UtmLinkDecoratorProps): React.ReactElement { const { children, ...options } = props const ref = useUtmLinkDecorator(options) - return <div ref={ref as React.RefObject<HTMLDivElement>}>{children}</div> + return <div ref={ref}>{children}</div> }src/inbound/form.ts (1)
111-121: Minor: Clarify the camelCase key stripping logic.The double replace
key.replace(/^utm_/, '').replace(/^utm/, '')handles both snake_case (utm_source→source) and camelCase (utmSource→Source). The subsequent.toLowerCase()normalises the result, but this could be simplified with a single regex.♻️ Proposed simplification
for (const [key, value] of Object.entries(params)) { if (value === undefined) continue - // Strip utm_ prefix to get short name - const shortName = key.replace(/^utm_/, '').replace(/^utm/, '') - if (shortName) { - // Handle snake_case: utm_source → source - shortNameMap.set(shortName.toLowerCase(), value) - } + // Strip utm_ or utm prefix (handles both snake_case and camelCase) + const shortName = key.replace(/^utm_?/i, '').toLowerCase() + if (shortName) { + shortNameMap.set(shortName, value) + } }README.md (1)
189-202: Clarify legacy signature deprecation status.The documentation shows both the new options object API and legacy positional arguments for
clearStoredUtmParameters. Consider adding a note indicating whether the legacy signatures are deprecated or will be maintained long-term, to help users understand which pattern to prefer.src/inbound/attribution.ts (1)
52-108: Consider adding a default case for defensive programming.The switch statement handles all current
AttributionModevalues ('last', 'first', 'both'), but lacks a default case. If a new mode is added to the type without updating this function, it would silently do nothing.🛡️ Proposed fix to add exhaustiveness check
case 'both': { // First-touch: write-once const hasFirstTouch = hasStoredUtmParameters(firstKey, storageType) if (!hasFirstTouch) { storeUtmParameters(params, { ...baseStorageOpts, storageKey: firstKey }) if (onStore) { try { onStore(params, { storageType, touch: 'first' }) } catch { // Callbacks must not break the pipeline } } } // Last-touch: always update storeUtmParameters(params, { ...baseStorageOpts, storageKey: lastKey }) if (onStore) { try { onStore(params, { storageType, touch: 'last' }) } catch { // Callbacks must not break the pipeline } } // Also write to main key (same as last-touch) storeUtmParameters(params, { ...baseStorageOpts, storageKey }) break } + + default: { + // Exhaustiveness check - ensures all modes are handled + const _exhaustiveCheck: never = attribution.mode + throw new Error(`Unknown attribution mode: ${_exhaustiveCheck}`) + } } }
| function autoCreateFields(forms: NodeListOf<Element>, params: UtmParameters): number { | ||
| let count = 0 | ||
| for (const form of forms) { | ||
| for (const [key, value] of Object.entries(params)) { | ||
| if (value === undefined) continue | ||
| const input = document.createElement('input') | ||
| input.type = 'hidden' | ||
| input.name = key | ||
| input.value = value | ||
| form.appendChild(input) | ||
| count++ | ||
| } | ||
| } | ||
| return count | ||
| } |
There was a problem hiding this comment.
autoCreateFields may create duplicate hidden inputs.
The function appends new hidden inputs without checking if fields with the same name already exist in the form. If called multiple times (e.g., on re-render or multiple invocations), this could result in duplicate form fields being submitted.
🛡️ Proposed fix to prevent duplicates
function autoCreateFields(forms: NodeListOf<Element>, params: UtmParameters): number {
let count = 0
for (const form of forms) {
for (const [key, value] of Object.entries(params)) {
if (value === undefined) continue
+ // Skip if field already exists
+ const existing = form.querySelector(`input[name="${key}"]`)
+ if (existing) continue
const input = document.createElement('input')
input.type = 'hidden'
input.name = key
input.value = value
form.appendChild(input)
count++
}
}
return count
}🤖 Prompt for AI Agents
In `@src/inbound/form.ts` around lines 140 - 154, The autoCreateFields function
currently appends hidden inputs blindly which can create duplicates; update it
to, for each form (forms) and each param key in UtmParameters, first check the
form for existing inputs with the same name (e.g., query the form for
input[name=key] or input[name="${key}"]) and if one exists update its value
instead of appending a new element, otherwise create and append the hidden
input; ensure the increment of count only happens when a new input is created
(not when updating an existing input) and keep references to the function name
autoCreateFields, the forms variable, and the input creation branch to locate
the change.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/inbound/form.ts`:
- Around line 66-82: The JSDoc for createUtmHiddenFields is inaccurate:
autoCreateFields counts both newly created and updated inputs, so update the
docstring to say it "Returns count of fields populated" (or similar) to reflect
that it includes updated fields, and ensure the comment above the function
references the same behavior; locate the createUtmHiddenFields function and
replace "Returns count of fields created" with "Returns count of fields
populated" (mentioning autoCreateFields) so docs match implementation.
In `@src/react/useUtmTracking.ts`:
- Around line 156-163: The clear() callback only removes the base storageKey but
not the attribution-specific keys, leaving firstTouchParams/lastTouchParams
stale; update clear() (and the clearStoredUtmParameters call it invokes) to also
remove storageKey + '_first' and storageKey + '_last' (and any other attribution
suffixes used by storeWithAttribution) so all attribution-related entries are
cleared, then call setUtmParameters(null) as before; reference the clear()
function, clearStoredUtmParameters helper, storeWithAttribution behavior, and
the storageKey/first/last suffixes to locate where to add removal of those extra
keys.
🧹 Nitpick comments (2)
src/react/useUtmTracking.ts (1)
241-269: Attribution mode logic is correct; consider passingonExpirefor consistency.The conditional logic for
firstTouchParamsandlastTouchParamscorrectly handles all three attribution modes. The use ofutmParametersas a dependency to force re-reads after capture is a valid pattern.Minor observation:
getStoredUtmParametersaccepts anonExpirecallback (per the relevant snippet insrc/common/storage.ts), but it's not passed here. If first-touch or last-touch data expires during retrieval, the configuredconfig.onExpirecallback won't be triggered. Consider whether this should be consistent with other storage reads.src/inbound/form.ts (1)
113-121: Consider simplifying the prefix-stripping logic.The double
replacehandles bothutm_source(snake_case) andutmSource(camelCase), which works correctly. A single regex could make the intent clearer, though this is a minor stylistic preference.♻️ Optional: unified regex
// Strip utm_ prefix to get short name - const shortName = key.replace(/^utm_/, '').replace(/^utm/, '') + const shortName = key.replace(/^utm_?/i, '')Note: Using the
iflag makes it case-insensitive, handling both formats in one pass.
…ed touch props, add guards - clear() in useUtmTracking now also removes first-touch and last-touch suffixed storage keys so attribution data doesn't go stale - Remove unused `touch` prop from FormPopulateOptions, LinkDecoratorOptions, UtmHiddenFieldsProps, and UseUtmFormDataOptions (dead code) - Add document.body null guard in observeAndDecorateLinks - Add exhaustiveness check (never) in attribution switch statement - Fix createUtmHiddenFields JSDoc to reflect update-or-create behavior 🤖 Generated with [Nori](https://nori.ai) 🤖 Generated with [Nori](https://nori.ai) Co-Authored-By: Nori <contact@tilework.tech>
1 participant
Summary
🤖 Generated with Nori
src/core/intosrc/inbound/,src/outbound/,src/common/— zero public API changesonCapture,onStore,onClear,onAppend,onExpirelifecycle hooks to config and core function optionsbuildUtmUrl()andvalidateUtmValues()for constructing UTM-tagged URLs with validation and warningspopulateFormFields,createUtmHiddenFields) and React (UtmHiddenFields,useUtmFormData) for injecting UTM data into formsdecorateLinks,observeAndDecorateLinks) and React (UtmLinkDecorator,useUtmLinkDecorator) for auto-appending UTM params to page links455 tests passing. No breaking changes — all new features are additive with backward-compatible defaults.
Test Plan
npm test)npm run type-check)npm run lint)npm run build)Share Nori with your team: https://www.npmjs.com/package/nori-ai
Summary by CodeRabbit
New Features
Documentation
Tests