-
Notifications
You must be signed in to change notification settings - Fork 1
refactor: route _fs_namespace reads through getFsNamespace helper #250
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
pablofullstory
wants to merge
1
commit into
main
Choose a base branch
from
refactor/fs-namespace-helper
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| /* eslint-disable no-underscore-dangle, import/prefer-default-export */ | ||
|
|
||
| /** | ||
| * Resolves the FullStory client namespace on `window`. | ||
| * | ||
| * Mirrors the resolution order used by `@fullstory/browser-api`: | ||
| * 1. The `data-fs-namespace` attribute on `document.currentScript` (best-effort). | ||
| * 2. The `_fs_namespace` global set on `window` by the FullStory snippet. | ||
| * 3. The default namespace `'FS'`. | ||
| * | ||
| * Note: `document.currentScript` is only meaningful while the helper module is being | ||
| * evaluated synchronously by a script tag. DLO calls this helper from deferred callbacks | ||
| * (appender log, operator handleData, lifecycle hook), so the attribute lookup is | ||
| * opportunistic and the global is the practical fallback floor — same behavior as the | ||
| * existing `(window as any)._fs_namespace` reads it replaces. | ||
| * | ||
| * @param win the global to read from; defaults to `window` | ||
| */ | ||
| export function getFsNamespace(win: any = window): string { | ||
| try { | ||
| const script = (win && win.document && win.document.currentScript) as HTMLScriptElement | null; | ||
| const attr = script && script.getAttribute && script.getAttribute('data-fs-namespace'); | ||
| if (attr) return attr; | ||
| } catch (_) { | ||
| // ignore: cross-origin or detached document access can throw | ||
| } | ||
| return (win && win._fs_namespace) || 'FS'; | ||
| } | ||
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,83 @@ | ||
| /* eslint-disable no-underscore-dangle, @typescript-eslint/no-explicit-any */ | ||
| import { expect } from 'chai'; | ||
| import 'mocha'; | ||
|
|
||
| import { getFsNamespace } from '../src/utils/fsNamespace'; | ||
|
|
||
| /** | ||
| * Builds a minimal fake `window`-like object with optional `_fs_namespace` and | ||
| * `document.currentScript` pieces so we can exercise the resolution order without | ||
| * mutating the real jsdom globals (and without coupling this spec to whatever | ||
| * other specs leave on the real `window`). | ||
| */ | ||
| function makeWin(opts: { | ||
| fsNamespace?: string; | ||
| scriptAttr?: string | null; | ||
| hasDocument?: boolean; | ||
| throwOnCurrentScript?: boolean; | ||
| } = {}): any { | ||
| const { | ||
| fsNamespace, scriptAttr, hasDocument = true, throwOnCurrentScript = false, | ||
| } = opts; | ||
|
|
||
| const script = scriptAttr === undefined ? null : { | ||
| getAttribute: (name: string) => (name === 'data-fs-namespace' ? scriptAttr : null), | ||
| }; | ||
|
|
||
| const win: any = {}; | ||
| if (fsNamespace !== undefined) { | ||
| win._fs_namespace = fsNamespace; | ||
| } | ||
| if (hasDocument) { | ||
| Object.defineProperty(win, 'document', { | ||
| get() { | ||
| if (throwOnCurrentScript) { | ||
| throw new Error('boom'); | ||
| } | ||
| return { currentScript: script }; | ||
| }, | ||
| }); | ||
| } | ||
| return win; | ||
| } | ||
|
|
||
| describe('getFsNamespace', () => { | ||
| it('returns the data-fs-namespace attribute when set on document.currentScript', () => { | ||
| const win = makeWin({ scriptAttr: 'CustomFS', fsNamespace: 'FS' }); | ||
| expect(getFsNamespace(win)).to.eq('CustomFS'); | ||
| }); | ||
|
|
||
| it('prefers the script attribute over _fs_namespace when both are present', () => { | ||
| const win = makeWin({ scriptAttr: 'AttrWins', fsNamespace: 'GlobalLoses' }); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| expect(getFsNamespace(win)).to.eq('AttrWins'); | ||
| }); | ||
|
|
||
| it('falls back to _fs_namespace when the script attribute is missing', () => { | ||
| const win = makeWin({ scriptAttr: null, fsNamespace: 'MyFS' }); | ||
| expect(getFsNamespace(win)).to.eq('MyFS'); | ||
| }); | ||
|
|
||
| it('falls back to _fs_namespace when document.currentScript is null (deferred call)', () => { | ||
| const win = makeWin({ fsNamespace: 'MyFS' }); | ||
| expect(getFsNamespace(win)).to.eq('MyFS'); | ||
| }); | ||
|
|
||
| it("falls back to 'FS' when neither the script attribute nor _fs_namespace are set", () => { | ||
| const win = makeWin({}); | ||
| expect(getFsNamespace(win)).to.eq('FS'); | ||
| }); | ||
|
|
||
| it('treats an empty-string script attribute as not set and falls through to the global', () => { | ||
| const win = makeWin({ scriptAttr: '', fsNamespace: 'MyFS' }); | ||
| expect(getFsNamespace(win)).to.eq('MyFS'); | ||
| }); | ||
|
|
||
| it('swallows errors from document access and falls back to the global', () => { | ||
| const win = makeWin({ throwOnCurrentScript: true, fsNamespace: 'MyFS' }); | ||
| expect(getFsNamespace(win)).to.eq('MyFS'); | ||
| }); | ||
|
|
||
| it("returns 'FS' when the global object is missing entirely", () => { | ||
| expect(getFsNamespace(undefined as any)).to.eq('FS'); | ||
| }); | ||
| }); | ||
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.

There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's say
fs.jsinstead of@fullstory/browser-apisince that's an internal package and this is theoretically an open source repo.