-
Notifications
You must be signed in to change notification settings - Fork 393
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
fix(ssr): fix expression scoping and other small fixes #4702
Conversation
@@ -348,6 +349,7 @@ export default function lwc(pluginOptions: RollupLwcOptions = {}): Plugin { | |||
enableStaticContentOptimization: pluginOptions.enableStaticContentOptimization, | |||
}), | |||
targetSSR, | |||
ssrMode, |
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.
This was not piped through before.
} from 'estree'; | ||
|
||
export const bImportHtmlEscape = esTemplate` | ||
import { htmlEscape } from '@lwc/shared'; | ||
import { htmlEscape } from '@lwc/ssr-runtime'; |
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.
It's simpler just to have everything at runtime imported from ssr-runtime, rather than introducing more dependencies at runtime.
@@ -43,7 +43,7 @@ | |||
} | |||
} | |||
}, | |||
"dependencies": { | |||
"devDependencies": { |
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.
Making this a devDep means that it's bundled into ssr-runtime
which just makes things simpler (e.g. swapping packages in Tachometer tests). Plus this is what we do for engine-dom
and engine-server
.
return cxt.isLocalVar(scopeReferencedId?.name) | ||
? expression | ||
: b.memberExpression(b.identifier('instance'), expression); | ||
} |
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.
Extracted out from for-each.ts
.
if (valueType === 'string') { | ||
yield \`="\${prefix}\${htmlEscape(attrOrPropValue, true)}"\`; | ||
yield \`="\${prefix}\${htmlEscape(attrValue, true)}"\`; |
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.
This is not a prop so don't call it a prop.
...ine-server/src/__tests__/fixtures/very-deeply-nested-each/modules/x/component/component.html
Outdated
Show resolved
Hide resolved
…-nested-each/modules/x/component/component.html Co-authored-by: Eugene Kashida <ekashida@gmail.com>
Details
Fixes attribute value expression rendering inside of loops (e.g.
for:each
). Also fixes some other minor unrelated stuff.Does this pull request introduce a breaking change?
Does this pull request introduce an observable change?