Skip to content
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

chore: cleanup and add tests #473

Merged
merged 1 commit into from
Nov 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 17 additions & 22 deletions src/lib/web-worker/worker-exec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,35 +118,30 @@ export const runScriptContent = (

/**
* Replace some `this` symbols with a new value.
* Still not perfect, but might be better than a more naive regex.
* Still not perfect, but might be better than a less advanced regex
* Check out the tests for examples: tests/unit/worker-exec.spec.ts
*
* This still fails with simple strings like:
* 'sadly we fail at this simple string'
*
* One way to do that would be to remove all comments from code and do single / double quote counting
* per symbol. But this will still fail with evals.
*/
export const replaceThisInSource = (scriptContent: string, newThis: string): string => {
// Best for now but not perfect
// Use a more complex regex to match potential preceding character and adjust replacement accordingly
const r0 = /(?<!([a-zA-Z0-9_$\.\'\"\`]))(\.\.\.)?this(?![a-zA-Z0-9_$:])/g;
const r2 = /([a-zA-Z0-9_$\.\'\"\`])?(\.\.\.)?this(?![a-zA-Z0-9_$:])/g;

return scriptContent.replace(r2, (match, p1, p2) => {
// console.log('\n');
// console.log('input: ' + scriptContent);
// console.log('match: ', match);
// console.log('p1: ', p1);
// console.log('p2: ', p2);
// console.log('\n');
/**
* Best for now but not perfect
* We don't use Regex lookbehind, because of Safari
*/
const FIND_THIS = /([a-zA-Z0-9_$\.\'\"\`])?(\.\.\.)?this(?![a-zA-Z0-9_$:])/g;

return scriptContent.replace(FIND_THIS, (match, p1, p2) => {
const prefix = (p1 || '') + (p2 || '');
if (p1 != null) {
return (p1 || '') + (p2 || '') + 'this';
return prefix + 'this';
}
// If there was a preceding character, include it unchanged
// console.log('===', scriptContent, '----', p1, p2);
return (p1 || '') + (p2 || '') + newThis;
return prefix + newThis;
});

// 3.5
// const r = /(^|[^a-zA-Z0-9_$.\'\"`])\.\.\.?this(?![a-zA-Z0-9_$:])/g;
// return scriptContent.replace(r, '$1' + newThis);
// const r = /(?<!([a-zA-Z0-9_$\.\'\"\`]))(\.\.\.)?this(?![a-zA-Z0-9_$:])/g;
// return scriptContent.replace(r, '$2' + newThis);
};

export const run = (env: WebWorkerEnvironment, scriptContent: string, scriptUrl?: string) => {
Expand Down
21 changes: 10 additions & 11 deletions tests/unit/worker-exec.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,6 @@ test('We should replace `this` keyword more or less sane', ({ env, win }) => {
};

// Should replace:
// assert.is(r('`sadly we fail at this simple string`'), '`sadly we fail at this simple string`');
assert.is(r('{this:123}'), '{this:123}');
assert.is(r('a.this'), 'a.this');
assert.is(r('[`kathis`]'), '[`kathis`]');
Expand All @@ -124,7 +123,7 @@ test('We should replace `this` keyword more or less sane', ({ env, win }) => {
a.b.this
let _this, This, $this
`;
// const rez = replaceThisInSource(input, `__this`);
const rez = replaceThisInSource(input, `__this`);

const out = `
// Should replace:
Expand All @@ -145,16 +144,16 @@ test('We should replace `this` keyword more or less sane', ({ env, win }) => {
let _this, This, $this
`;

// assert.is(rez, out);
assert.is(rez, out);
});

// test('properly replaces this is js window context', ({ env, win }) => {
// const s = `
// let a = { this: 123 };
// window.result = a.this;
// `;
// run(env, s);
// assert.is(win.result, 123);
// });
test('properly replaces this is js window context', ({ env, win }) => {
const s = `
let a = { this: 123 };
window.result = a.this;
`;
run(env, s);
assert.is(win.result, 123);
});

test.run();