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: improve let block scoping #81

Merged
merged 1 commit into from
Jan 31, 2024
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
26 changes: 22 additions & 4 deletions plugins/converter.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { expect, test, describe, beforeAll, beforeEach } from 'vitest';
import { expect, test, describe, beforeAll, beforeEach, afterEach } from 'vitest';
import { preprocess } from '@glimmer/syntax';

import { ComplexJSType, convert } from './converter';
Expand Down Expand Up @@ -463,17 +463,35 @@ describe.each([
});
});
describe('let condition', () => {
const mathRandom = Math.random;
beforeEach(() => {
Math.random = () => 0.001;
});
afterEach(() => {
Math.random = mathRandom;
});
test('it works', () => {
expect(
$t<ASTv1.BlockStatement>(
`{{#let foo "name" as |bar k|}}p{{bar}}{{k}}{{/let}}`,
),
).toEqual(
`$:...(() => {let bar = $:() => $:foo;let k = "name";return [$_text("p"), ${
flags.IS_GLIMMER_COMPAT_MODE ? '() => bar' : 'bar'
}, ${flags.IS_GLIMMER_COMPAT_MODE ? '() => k' : 'k'}]})()`,
`$:...(() => {let self = this;let Let_bar_6c3gez6 = $:() => $:foo;let Let_k_6c3gez6 = "name";return [$_text("p"), ${
flags.IS_GLIMMER_COMPAT_MODE ? '() => Let_bar_6c3gez6' : 'Let_bar_6c3gez6'
}, ${flags.IS_GLIMMER_COMPAT_MODE ? '() => Let_k_6c3gez6' : 'Let_k_6c3gez6'}]})()`,
);
});
test('it not override arg assign case', () => {
const result = $t<ASTv1.BlockStatement>(
`{{#let foo "name" as |bar k|}}<Div @bar={{bar}} bar={{if bar bar}} />{{/let}}`,
);
if (flags.IS_GLIMMER_COMPAT_MODE) {
expect(result).toEqual(`$:...(() => {let self = this;let Let_bar_6c3gez6 = $:() => $:foo;let Let_k_6c3gez6 = "name";return [$_c(Div,$_args({bar: () => Let_bar_6c3gez6},{},[[],[['bar', () => $:$__if(Let_bar_6c3gez6,Let_bar_6c3gez6)]],[]]), this)]})()`);
} else {
expect(result).toEqual(`$:...(() => {let self = this;let Let_bar_6c3gez6 = $:() => $:foo;let Let_k_6c3gez6 = "name";return [$_c(Div,{bar: Let_bar_6c3gez6, "$:[$PROPS_SYMBOL]": [[],[['bar', () => $:$__if(Let_bar_6c3gez6,Let_bar_6c3gez6)]],[]]}, this)]})()`);
}

})
});
describe('each condition', () => {
test('it adds unstable child wrapper for simple multi-nodes', () => {
Expand Down
46 changes: 33 additions & 13 deletions plugins/converter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -264,37 +264,57 @@ export function convert(seenNodes: Set<ASTv1.Node>, flags: Flags) {
key: keyValue,
} as HBSControlExpression;
} else if (name === 'let') {
const varScopeName = Math.random().toString(36).substring(7);
const namesToReplace: Record<string, string> = {};
const vars = node.params.map((p, index) => {
let isSubExpression = p.type === 'SubExpression';
let isString = p.type === 'StringLiteral';
let isBoolean = p.type === 'BooleanLiteral';
let isNull = p.type === 'NullLiteral';
let isUndefined = p.type === 'UndefinedLiteral';
let originalName = node.program.blockParams[index];
let newName = `Let_${originalName}_${varScopeName}`;
namesToReplace[originalName] = `${newName}`;
if (
isSubExpression ||
isString ||
isBoolean ||
isNull ||
isUndefined
) {
return `let ${node.program.blockParams[index]} = ${ToJSType(
p,
false,
)};`;
return `let ${newName} = ${ToJSType(p, false)};`;
} else {
return `let ${node.program.blockParams[index]} = $:() => ${ToJSType(
p,
)};`;
return `let ${newName} = $:() => ${ToJSType(p)};`;
}
});
// note, at the moment nested let's works fine if no name overlap,
// looks like fix for other case should be on babel level;
const result = `$:...(() => {${vars.join(
'',
)}return [${serializeChildren(
children as unknown as [string | HBSNode | HBSControlExpression],
'this', // @todo - fix possible context floating here
)}]})()`;
// @todo - likely should be a babel work
function fixChildScopes(str: string) {
// console.log('fixChildScopes', str, JSON.stringify(namesToReplace));
Object.keys(namesToReplace).forEach((key) => {
/*
allow: {{name}} {{foo name}} name.bar
don't allow:
name:
name=
foo.name
'name'
"name"
*/
const re = new RegExp(`(?<!\\.)\\b${key}\\b(?!(=|'|\"|:)[^ ]*)`, 'g');
str = str.replace(re, namesToReplace[key]);
});
return str;
}
const result = `$:...(() => {let self = this;${vars
.join('')
.split('this.')
.join('self.')}return [${fixChildScopes(
serializeChildren(
children as unknown as [string | HBSNode | HBSControlExpression],
'this', // @todo - fix possible context floating here
) )}]})()`;
return result;
}

Expand Down
Loading