Skip to content

Commit

Permalink
refactor(toolbox/getcontext): refactoring to not throw, but instead l…
Browse files Browse the repository at this point in the history
…og errors
  • Loading branch information
korgon committed Dec 23, 2024
1 parent b3cf7ce commit 02f612a
Show file tree
Hide file tree
Showing 2 changed files with 82 additions and 60 deletions.
119 changes: 70 additions & 49 deletions packages/snap-toolbox/src/getContext/getContext.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -301,25 +301,47 @@ describe('getContext', () => {
}).not.toThrow();
});

it('throws an error when JavaScript keywords are provided in the evaluate array', () => {
it('logs an error when there is invalid syntax in the script context', () => {
const scriptTag = document.createElement('script');
scriptTag.setAttribute('type', 'searchspring');
scriptTag.innerHTML = `
valid = 'valid';
invalid = syntax error;
`;

// invalid param that should throw
expect(() => {
getContext(['class'], scriptTag);
}).toThrow('getContext: JavaScript keywords are not allowed in evaluate array');
const consoleError = jest.spyOn(console, 'error');

expect(() => {
getContext(['const'], scriptTag);
}).toThrow('getContext: JavaScript keywords are not allowed in evaluate array');
const context = getContext(['valid', 'invalid'], scriptTag);
expect(consoleError).toHaveBeenCalledWith("getContext: error evaluating 'valid'");
expect(consoleError).toHaveBeenCalledWith("getContext: error evaluating 'invalid'");
expect(context).toStrictEqual({});
}).not.toThrow();

consoleError.mockRestore();
});

it('does not throw an error when keywords are provided in the evaluate array, but logs an error', () => {
const scriptTag = document.createElement('script');
scriptTag.setAttribute('type', 'searchspring');

const consoleError = jest.spyOn(console, 'error');

expect(() => {
getContext(['if'], scriptTag);
}).toThrow('getContext: JavaScript keywords are not allowed in evaluate array');
// invalid param that should generate an error - `getContext: JavaScript keyword found: '${item}'! Please use a different variable name.`
getContext(['class', 'const', 'if', 'valid'], scriptTag);
expect(consoleError).toHaveBeenCalledWith("getContext: JavaScript keyword found: 'class'! Please use a different variable name.");
expect(consoleError).toHaveBeenCalledWith("getContext: JavaScript keyword found: 'const'! Please use a different variable name.");
expect(consoleError).toHaveBeenCalledWith("getContext: JavaScript keyword found: 'if'! Please use a different variable name.");
expect(consoleError).not.toHaveBeenCalledWith("getContext: JavaScript keyword found: 'valid'! Please use a different variable name.");

expect(consoleError).toHaveBeenCalledTimes(3);
}).not.toThrow();

consoleError.mockRestore();
});

it('throws an error when JavaScript keywords are found in script inner variables', () => {
it('does not throw when keywords are using in inner script variables but logs an error and returns an empty context', () => {
const scriptTag = document.createElement('script');
scriptTag.setAttribute('type', 'searchspring');
scriptTag.innerHTML = `
Expand All @@ -329,9 +351,45 @@ describe('getContext', () => {
validVar = "should-evaluate";
`;

const consoleError = jest.spyOn(console, 'error');

expect(() => {
getContext(['validVar'], scriptTag);
}).toThrow('getContext: JavaScript keywords cannot be used as variable names in script');
const context = getContext(['validVar'], scriptTag);

expect(consoleError).toHaveBeenCalledWith("getContext: JavaScript keyword found: 'class'! Please use a different variable name.");
expect(consoleError).toHaveBeenCalledWith("getContext: JavaScript keyword found: 'const'! Please use a different variable name.");
expect(consoleError).toHaveBeenCalledWith("getContext: JavaScript keyword found: 'if'! Please use a different variable name.");
expect(consoleError).not.toHaveBeenCalledWith("getContext: JavaScript keyword found: 'validVar'! Please use a different variable name.");

expect(consoleError).toHaveBeenCalledWith("getContext: error evaluating 'validVar'");

// logs above errors plus the actual error when attempting to evaluate "validVar"
expect(consoleError).toHaveBeenCalledTimes(5);

expect(context).toStrictEqual({});
}).not.toThrow();

consoleError.mockRestore();
});

it('allows javascript keywords in object properties and string values', () => {
const scriptTag = document.createElement('script');
scriptTag.setAttribute('type', 'searchspring');
scriptTag.innerHTML = `
config = {
class: "class",
const: "const",
if: true
};
`;

const vars = getContext(['config'], scriptTag);
expect(vars).toHaveProperty('config');
expect(vars.config).toEqual({
class: 'class',
const: 'const',
if: true,
});
});
});

Expand Down Expand Up @@ -402,40 +460,3 @@ describe('variable name parsing', () => {
expect(vars).not.toHaveProperty('nested');
});
});

describe('javascript keywords', () => {
it('filters out javascript keywords from evaluation', () => {
const scriptTag = document.createElement('script');
scriptTag.setAttribute('type', 'searchspring');
scriptTag.innerHTML = `
class = "should-not-evaluate";
const = "should-not-evaluate";
if = "should-not-evaluate";
validVar = "should-evaluate";
`;

expect(() => {
const vars = getContext(['class', 'const', 'if', 'validVar'], scriptTag);
}).toThrow('getContext: JavaScript keywords are not allowed in evaluate array');
});

it('allows javascript keywords in object properties and string values', () => {
const scriptTag = document.createElement('script');
scriptTag.setAttribute('type', 'searchspring');
scriptTag.innerHTML = `
config = {
class: "my-class",
const: "my-const",
if: true
};
`;

const vars = getContext(['config'], scriptTag);
expect(vars).toHaveProperty('config');
expect(vars.config).toEqual({
class: 'my-class',
const: 'my-const',
if: true,
});
});
});
23 changes: 12 additions & 11 deletions packages/snap-toolbox/src/getContext/getContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,6 @@ const JAVASCRIPT_KEYWORDS = new Set([
]);

export function getContext(evaluate: string[] = [], script?: HTMLScriptElement | string): ContextVariables {
if (evaluate?.some((name) => JAVASCRIPT_KEYWORDS.has(name))) {
throw new Error('getContext: JavaScript keywords are not allowed in evaluate array');
}

if (!script || typeof script === 'string') {
const scripts = Array.from(document.querySelectorAll((script as string) || 'script[id^=searchspring], script[src*="snapui.searchspring.io"]'));

Expand Down Expand Up @@ -106,15 +102,16 @@ export function getContext(evaluate: string[] = [], script?: HTMLScriptElement |
.match(/([a-zA-Z_$][a-zA-Z_$0-9]*)\s*=/g)
?.map((match) => match.replace(/[\s=]/g, ''));

if (scriptInnerVars?.some((name) => JAVASCRIPT_KEYWORDS.has(name))) {
throw new Error('getContext: JavaScript keywords cannot be used as variable names in script');
}

const combinedVars = evaluate.concat(scriptInnerVars || []);

// de-dupe vars
const evaluateVars = combinedVars.filter((item, index) => {
return combinedVars.indexOf(item) === index && !JAVASCRIPT_KEYWORDS.has(item);
const isKeyword = JAVASCRIPT_KEYWORDS.has(item);
// console error if keyword
if (isKeyword) {
console.error(`getContext: JavaScript keyword found: '${item}'! Please use a different variable name.`);
}
return combinedVars.indexOf(item) === index && !isKeyword;
});

// evaluate text and put into variables
Expand All @@ -126,9 +123,13 @@ export function getContext(evaluate: string[] = [], script?: HTMLScriptElement |
return ${name};
`);
scriptVariables[name] = fn();
} catch (e) {
} catch (err) {
// if evaluation fails, set to undefined
console.log(`getContext: error evaluating ${name}`);
const isKeyword = JAVASCRIPT_KEYWORDS.has(name);
if (!isKeyword) {
console.error(`getContext: error evaluating '${name}'`);
console.error(err);
}
scriptVariables[name] = undefined;
}
});
Expand Down

0 comments on commit 02f612a

Please sign in to comment.