-
-
Notifications
You must be signed in to change notification settings - Fork 52
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
Sloppy string replacements #62
Comments
We need to stop replacing component names found in strings. With regex you can't simply make sure there isn't a Based on how SSR components are created, it looks like it might be even more important to check for the backtick syntax in string literals. See SSR component for layout/scripts/stores.svelte/* generated by Svelte v3.29.4 */
/*import {
create_ssr_component,
each,
escape,
missing_component,
validate_component
} from "svelte/internal";*/
/*import layout_components_template_svelte from "../components/template.svelte";*/
// Svelte store example:
/*import { count } from "../scripts/stores.svelte";*/
/*import Incrementer from "../components/incrementer.svelte";*/
/*import Decrementer from "../components/decrementer.svelte";*/
var layout_content_blog_svelte = create_ssr_component(($$result, $$props, $$bindings, slots) => {
let { title } = $$props,
{ body } = $$props,
{ author } = $$props,
{ date } = $$props,
{ store } = $$props;
let count_value;
const unsubscribe = count.subscribe(value => {
count_value = value;
});
let { components } = $$props, { allComponents } = $$props;
if ($$props.title === void 0 && $$bindings.title && title !== void 0) $$bindings.title(title);
if ($$props.body === void 0 && $$bindings.body && body !== void 0) $$bindings.body(body);
if ($$props.author === void 0 && $$bindings.author && author !== void 0) $$bindings.author(author);
if ($$props.date === void 0 && $$bindings.date && date !== void 0) $$bindings.date(date);
if ($$props.store === void 0 && $$bindings.store && store !== void 0) $$bindings.store(store);
if ($$props.components === void 0 && $$bindings.components && components !== void 0) $$bindings.components(components);
if ($$props.allComponents === void 0 && $$bindings.allComponents && allComponents !== void 0) $$bindings.allComponents(allComponents);
return `<h1>${escape(title)}</h1>
<p><em>${author ? `Written by ${escape(author)}` : ``}${date ? ` on ${escape(date)}` : ``}</em></p>
<div>${each(body, paragraph => `<p>${paragraph}</p>`)}</div>
${store
? `<h3>The count is ${escape(count_value)}</h3>
${validate_component(Incrementer, "Incrementer").$$render($$result, {}, {}, {})}
${validate_component(Decrementer, "Decrementer").$$render($$result, {}, {}, {})}`
: ``}
${components
? `${each(components, ({ title, component, fields }) => `<p>${escape(title)}</p>
${validate_component(allComponents["layout_components_" + component + "_svelte"] || missing_component, "svelte:component").$$render($$result, Object.assign(fields), {}, {})}`)}`
: ``}
${validate_component(layout_components_template_svelte, "layout_components_template_svelte").$$render($$result, { type: "blog" }, {}, {})}
<p><a href="${"/"}">Back home</a></p>`;
});
/*export default Component;*/ Need to stop the word "count" from getting replaced ^ |
I think we only need to replace named variables with component signatures in the template literal that is |
Added some code that targets placeholders within template literals and also makes sure not to confuse components that are similarly name (e.g. Regexp snippetreTemplatePlaceholder := regexp.MustCompile(`(\$\{).*(\})`)
reImportNameUse := regexp.MustCompile(`([^a-zA-Z_0-9])` + importNameStr + `([^a-zA-Z_0-9])`)
ssrStr = reTemplatePlaceholder.ReplaceAllStringFunc(ssrStr,
func(placeholder string) string {
return reImportNameUse.ReplaceAllString(placeholder, "${1}"+importSignature+"${2}")
},
) This works in most cases, but there are still instances where it doesn't do the replacement correctly, such as when a template literal replacement spans multiple lines. In the following example Multiline template literal replacement/* generated by Svelte v3.29.4 */
/*import {
create_ssr_component,
each,
escape,
validate_component
} from "svelte/internal";*/
/*import Grid from "../components/grid.svelte";*/
/*import Uses from "../components/template.svelte";*/
var layout_content_index_svelte = create_ssr_component(($$result, $$props, $$bindings, slots) => {
let { title } = $$props,
{ intro } = $$props,
{ components } = $$props,
{ allContent } = $$props;
if ($$props.title === void 0 && $$bindings.title && title !== void 0) $$bindings.title(title);
if ($$props.intro === void 0 && $$bindings.intro && intro !== void 0) $$bindings.intro(intro);
if ($$props.components === void 0 && $$bindings.components && components !== void 0) $$bindings.components(components);
if ($$props.allContent === void 0 && $$bindings.allContent && allContent !== void 0) $$bindings.allContent(allContent);
return `<h1>${escape(title)}</h1>
<section id="${"intro"}">${each(intro, paragraph => `<p>${paragraph}</p>`)}</section>
<div><h3>Recent blog posts:</h3>
${validate_component(Grid, "Grid").$$render(
$$result,
{
items: allContent.filter(content => content.type == "blog")
},
{},
{}
)}
<br></div>
${validate_component(layout_components_template_svelte, "layout_components_template_svelte").$$render($$result, { type: "index" }, {}, {})}`;
});
/*export default Component;*/ There are other times, like in script only components where the replacements don't actually happen in the template literals that are Incrementer component/* generated by Svelte v3.29.4 */
/*import { create_ssr_component } from "svelte/internal";*/
/*import { count } from "../scripts/stores.svelte";*/
var layout_components_incrementer_svelte = create_ssr_component(($$result, $$props, $$bindings, slots) => {
function increment() {
count.update(n => n + 1);
}
return `<button>+
</button>`;
});
/*export default Component;*/ |
More issues that seem to be related to nested placeholders in JS template strings:
Example nested placeholders/* generated by Svelte v3.29.4 */
/*import {
create_ssr_component,
each,
escape,
missing_component,
validate_component
} from "svelte/internal";*/
/*import Uses from "../components/template.svelte";*/
// Svelte store example:
/*import { count } from "../scripts/stores.svelte";*/
/*import Incrementer from "../components/incrementer.svelte";*/
/*import Decrementer from "../components/decrementer.svelte";*/
var layout_content_blog_svelte = create_ssr_component(($$result, $$props, $$bindings, slots) => {
let { title } = $$props,
{ body } = $$props,
{ author } = $$props,
{ date } = $$props,
{ store } = $$props;
let count_value;
const unsubscribe = count.subscribe(value => {
count_value = value;
});
let { components } = $$props, { allComponents } = $$props;
if ($$props.title === void 0 && $$bindings.title && title !== void 0) $$bindings.title(title);
if ($$props.body === void 0 && $$bindings.body && body !== void 0) $$bindings.body(body);
if ($$props.author === void 0 && $$bindings.author && author !== void 0) $$bindings.author(author);
if ($$props.date === void 0 && $$bindings.date && date !== void 0) $$bindings.date(date);
if ($$props.store === void 0 && $$bindings.store && store !== void 0) $$bindings.store(store);
if ($$props.components === void 0 && $$bindings.components && components !== void 0) $$bindings.components(components);
if ($$props.allComponents === void 0 && $$bindings.allComponents && allComponents !== void 0) $$bindings.allComponents(allComponents);
return `<h1>${escape(title)}</h1>
<p><em>${author ? `Written by ${escape(author)}` : ``}${date ? ` on ${escape(date)}` : ``}</em></p>
<div>${each(body, paragraph => `<p>${paragraph}</p>`)}</div>
${store
? `<h3>The layout_scripts_stores_svelte is ${escape(count_value)}</h3>
${validate_component(Incrementer, "Incrementer").$$render($$result, {}, {}, {})}
${validate_component(Decrementer, "Decrementer").$$render($$result, {}, {}, {})}`
: ``}
${components
? `${each(components, ({ title, component, fields }) => `<p>${escape(title)}</p>
${validate_component(allComponents["layout_components_" + component + "_svelte"] || missing_component, "svelte:component").$$render($$result, Object.assign(fields), {}, {})}`)}`
: ``}
${validate_component(layout_components_template_svelte, "layout_components_template_svelte").$$render($$result, { type: "blog" }, {}, {})}
<p><a href="${"/"}">Back home</a></p>`;
});
/*export default Component;*/ |
I updated the regexp for SSR component signature replacements to target Not defined errors
Those variables previously were replaced by component signatures: Since the SSR components are only used to create static HTML, it might not be necessary to replace these script variables at all. We are doing some extra processing behind the scenes, that we can probably remove. Basically script only SSR components don't Go processing for script only componentsnamedExports := reStaticExport.FindAllStringSubmatch(ssrStr, -1)
// Loop through all export statements.
for _, namedExport := range namedExports {
// Get exported functions that aren't default.
if !strings.HasPrefix(namedExport[1], "default ") {
exportName := strings.Trim(namedExport[1], "{ }")
if exportName != "" && componentSignature != "" {
if strings.Contains(ssrStr, "return ``;") {
ssrStr = strings.ReplaceAll(ssrStr, componentSignature, componentSignature+"_NOT_USED")
}
ssrStr = strings.ReplaceAll(ssrStr, exportName, componentSignature)
}
}
} Example script only SSR components: layout_scripts_make_title_svelte/* generated by Svelte v3.29.4 */
/*import { create_ssr_component } from "svelte/internal";*/
var layout_scripts_make_title_svelte = filename => {
if (filename == "index.json") {
return "Home";
} else if (filename) {
// Remove file extension.
filename = filename.split(".").slice(0, -1).join(".");
// Convert underscores and hyphens to spaces.
filename = filename.replace(/_|-/g, " ");
// Capitalize first letter of each word.
filename = filename.split(" ").map(s => s.charAt(0).toUpperCase() + s.substring(1)).join(" ");
}
return filename;
};
var layout_scripts_make_title_svelte_NOT_USED = create_ssr_component(($$result, $$props, $$bindings, slots) => {
return ``;
});
/*export default Component;*/
/*export { layout_scripts_make_title_svelte };*/ layout_scripts_sort_by_date_svelte/* generated by Svelte v3.29.4 */
/*import { create_ssr_component } from "svelte/internal";*/
var layout_scripts_sort_by_date_svelte = (items, order) => {
items.sort((a, b) => {
// Must have a field specifically named "date" to work.
// Feel free to extend to other custom named date fields.
if (a.hasOwnProperty("fields") && b.hasOwnProperty("fields")) {
if (a.fields.hasOwnProperty("date") && b.fields.hasOwnProperty("date")) {
let aDate = new Date(a.fields.date);
let bDate = new Date(b.fields.date);
if (order == "oldest") {
return aDate - bDate;
}
return bDate - aDate;
}
}
});
return items;
};
var layout_scripts_sort_by_date_svelte_NOT_USED = create_ssr_component(($$result, $$props, $$bindings, slots) => {
return ``;
});
/*export default Component;*/
/*export { layout_scripts_sort_by_date_svelte };*/ layout_scripts_stores_svelte/* generated by Svelte v3.29.4 */
/*import { create_ssr_component } from "svelte/internal";*/
/*import { writable } from "svelte/store";*/
var layout_scripts_stores_svelte = writable(0);
var layout_scripts_stores_svelte_NOT_USED = create_ssr_component(($$result, $$props, $$bindings, slots) => {
return ``;
});
/*export default Component;*/
/*export { layout_scripts_stores_svelte };*/ Defining each script with a component signature was necessary to avoid name conflicts across files because once they are all put in the same v8 context, you lose those clear boundaries. However if the script logic isn't used in the creating of static HTML anyways, we could leave the original variable names so v8go doesn't error out when looking for them. It won't matter if variables of the same name override each other since they aren't actually used anyway. |
I take it back, layout_global_html_svelte (Before import variable replacement)/* generated by Svelte v3.29.4 */
/*import {
create_ssr_component,
missing_component,
validate_component
} from "svelte/internal";*/
/*import Head from "./head.svelte";*/
/*import Nav from "./nav.svelte";*/
/*import Footer from "./footer.svelte";*/
/*import { makeTitle } from "../scripts/make_title.svelte";*/
var css = {
code: "html.svelte-196y134,body.svelte-196y134{height:100%}body.svelte-196y134{font-family:'Rubik', sans-serif;display:flex;flex-direction:column;margin:0}main.svelte-196y134{flex:1 0 auto}.container{max-width:1024px;margin:0 auto;flex-grow:1;padding:0 20px}:root{--primary:rgb(34, 166, 237);--primary-dark:rgb(16, 92, 133);--accent:rgb(254, 211, 48);--base:rgb(245, 245, 245);--base-dark:rgb(17, 17, 17)}main a{position:relative;text-decoration:none;color:var(--base-dark);padding-bottom:5px}main a:before{content:\"\";width:100%;height:100%;background-image:linear-gradient(to top, var(--accent) 25%, rgba(0, 0, 0, 0) 40%);position:absolute;left:0;bottom:2px;z-index:-1;will-change:width;transform:rotate(-2deg);transform-origin:left bottom;transition:width .1s ease-out}main a:hover:before{width:0;transition-duration:.15s}",
map: "{\"version\":3,\"file\":null,\"sources\":[null],\"sourcesContent\":[\"<script>\\n /*import Head from './head.svelte';\\n import Nav from './nav.svelte';\\n import Footer from './footer.svelte';\\n import { makeTitle } from '../scripts/make_title.svelte';\\n\\n /*export let route, content, allContent, allComponents;\\n</script>\\n\\n<html lang=\\\"en\\\">\\n<Head title={makeTitle(content.filename)} />\\n<body>\\n <Nav />\\n <main>\\n <div class=\\\"container\\\">\\n <svelte:component this={route} {...content.fields} {allContent} {allComponents} />\\n <br />\\n </div>\\n </main>\\n <Footer {allContent} />\\n</body>\\n</html>\\n\\n<style>\\n html, body {\\n height: 100%;\\n }\\n\\n body {\\n font-family: 'Rubik', sans-serif;\\n display: flex;\\n flex-direction: column;\\n margin: 0;\\n }\\n main {\\n flex: 1 0 auto;\\n }\\n :global(.container) {\\n max-width: 1024px;\\n margin: 0 auto;\\n flex-grow: 1;\\n padding: 0 20px;\\n }\\n :global(:root) {\\n --primary: rgb(34, 166, 237);\\n --primary-dark: rgb(16, 92, 133);\\n --accent: rgb(254, 211, 48);\\n --base: rgb(245, 245, 245);\\n --base-dark: rgb(17, 17, 17);\\n }\\n :global(main a) {\\n position: relative;\\n text-decoration: none;\\n color: var(--base-dark);\\n padding-bottom: 5px;\\n }\\n :global(main a:before) {\\n content: \\\"\\\";\\n width: 100%;\\n height: 100%;\\n background-image: linear-gradient(to top, var(--accent) 25%, rgba(0, 0, 0, 0) 40%); \\n position: absolute;\\n left: 0;\\n bottom: 2px;\\n z-index: -1; \\n will-change: width;\\n transform: rotate(-2deg);\\n transform-origin: left bottom;\\n transition: width .1s ease-out;\\n }\\n :global(main a:hover:before) {\\n width: 0;\\n transition-duration: .15s;*/*/\\n }\\n</style>\\n\"],\"names\":[],\"mappings\":\"AAwBE,mBAAI,CAAE,IAAI,eAAC,CAAC,AACV,MAAM,CAAE,IAAI,AACd,CAAC,AAED,IAAI,eAAC,CAAC,AACJ,WAAW,CAAE,OAAO,CAAC,CAAC,UAAU,CAChC,OAAO,CAAE,IAAI,CACb,cAAc,CAAE,MAAM,CACtB,MAAM,CAAE,CAAC,AACX,CAAC,AACD,IAAI,eAAC,CAAC,AACJ,IAAI,CAAE,CAAC,CAAC,CAAC,CAAC,IAAI,AAChB,CAAC,AACO,UAAU,AAAE,CAAC,AACnB,SAAS,CAAE,MAAM,CACjB,MAAM,CAAE,CAAC,CAAC,IAAI,CACd,SAAS,CAAE,CAAC,CACZ,OAAO,CAAE,CAAC,CAAC,IAAI,AACjB,CAAC,AACO,KAAK,AAAE,CAAC,AACd,SAAS,CAAE,iBAAiB,CAC5B,cAAc,CAAE,gBAAgB,CAChC,QAAQ,CAAE,iBAAiB,CAC3B,MAAM,CAAE,kBAAkB,CAC1B,WAAW,CAAE,eAAe,AAC9B,CAAC,AACO,MAAM,AAAE,CAAC,AACf,QAAQ,CAAE,QAAQ,CAClB,eAAe,CAAE,IAAI,CACrB,KAAK,CAAE,IAAI,WAAW,CAAC,CACvB,cAAc,CAAE,GAAG,AACrB,CAAC,AACO,aAAa,AAAE,CAAC,AACtB,OAAO,CAAE,EAAE,CACX,KAAK,CAAE,IAAI,CACX,MAAM,CAAE,IAAI,CACZ,gBAAgB,CAAE,gBAAgB,EAAE,CAAC,GAAG,CAAC,CAAC,IAAI,QAAQ,CAAC,CAAC,GAAG,CAAC,CAAC,KAAK,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,GAAG,CAAC,CAClF,QAAQ,CAAE,QAAQ,CAClB,IAAI,CAAE,CAAC,CACP,MAAM,CAAE,GAAG,CACX,OAAO,CAAE,EAAE,CACX,WAAW,CAAE,KAAK,CAClB,SAAS,CAAE,OAAO,KAAK,CAAC,CACxB,gBAAgB,CAAE,IAAI,CAAC,MAAM,CAC7B,UAAU,CAAE,KAAK,CAAC,GAAG,CAAC,QAAQ,AAChC,CAAC,AACO,mBAAmB,AAAE,CAAC,AAC5B,KAAK,CAAE,CAAC,CACR,mBAAmB,CAAE,IAAI,AAC3B,CAAC\"}"
};
var layout_global_html_svelte = create_ssr_component(($$result, $$props, $$bindings, slots) => {
let { route } = $$props,
{ content } = $$props,
{ allContent } = $$props,
{ allComponents } = $$props;
if ($$props.route === void 0 && $$bindings.route && route !== void 0) $$bindings.route(route);
if ($$props.content === void 0 && $$bindings.content && content !== void 0) $$bindings.content(content);
if ($$props.allContent === void 0 && $$bindings.allContent && allContent !== void 0) $$bindings.allContent(allContent);
if ($$props.allComponents === void 0 && $$bindings.allComponents && allComponents !== void 0) $$bindings.allComponents(allComponents);
$$result.css.add(css);
return `<html lang="${"en"}" class="${"svelte-196y134"}">${validate_component(Head, "Head").$$render($$result, { title: makeTitle(content.filename) }, {}, {})}
<body class="${"svelte-196y134"}">${validate_component(Nav, "Nav").$$render($$result, {}, {}, {})}
<main class="${"svelte-196y134"}"><div class="${"container"}">${validate_component(route || missing_component, "svelte:component").$$render($$result, Object.assign(content.fields, { allContent }, { allComponents }), {}, {})}
<br></div></main>
${validate_component(Footer, "Footer").$$render($$result, { allContent }, {}, {})}</body>
</html>`;
});
/*export default Component;*/ It's hard to see from the snippet above, but after import variable replacement you'd get an error like this:
|
^ that brings up problems with our handling of module exports in general. When the default started uses |
Instead of trying to figure out with regexp which text is a variable and which isn't for replacing exported modules, maybe it'll just be easier to revise the import statement. So if we find something like this: /*import { makeTitle } from "../scripts/make_title.svelte";*/ Add an line below like this: var makeTitle = layout_scripts_make_title_svelte_makeTitle; This would queue off the updated module exports that now look like this: makeTitle/* generated by Svelte v3.29.4 */
/*import { create_ssr_component } from "svelte/internal";*/
var layout_scripts_make_title_svelte_makeTitle = filename => {
if (filename == "index.json") {
return "Home";
} else if (filename) {
// Remove file extension.
filename = filename.split(".").slice(0, -1).join(".");
// Convert underscores and hyphens to spaces.
filename = filename.replace(/_|-/g, " ");
// Capitalize first letter of each word.
filename = filename.split(" ").map(s => s.charAt(0).toUpperCase() + s.substring(1)).join(" ");
}
return filename;
};
var layout_scripts_make_title_svelte = create_ssr_component(($$result, $$props, $$bindings, slots) => {
return ``;
});
/*export default Component;*/
/*export { layout_scripts_make_title_svelte_makeTitle };*/ sortByDate/* generated by Svelte v3.29.4 */
/*import { create_ssr_component } from "svelte/internal";*/
var layout_scripts_sort_by_date_svelte_sortByDate = (items, order) => {
items.sort((a, b) => {
// Must have a field specifically named "date" to work.
// Feel free to extend to other custom named date fields.
if (a.hasOwnProperty("fields") && b.hasOwnProperty("fields")) {
if (a.fields.hasOwnProperty("date") && b.fields.hasOwnProperty("date")) {
let aDate = new Date(a.fields.date);
let bDate = new Date(b.fields.date);
if (order == "oldest") {
return aDate - bDate;
}
return bDate - aDate;
}
}
});
return items;
};
var layout_scripts_sort_by_date_svelte = create_ssr_component(($$result, $$props, $$bindings, slots) => {
return ``;
});
/*export default Component;*/
/*export { layout_scripts_sort_by_date_svelte_sortByDate };*/ count/* generated by Svelte v3.29.4 */
/*import { create_ssr_component } from "svelte/internal";*/
/*import { writable } from "svelte/store";*/
var layout_scripts_stores_svelte_count = writable(0);
var layout_scripts_stores_svelte = create_ssr_component(($$result, $$props, $$bindings, slots) => {
return ``;
});
/*export default Component;*/
/*export { layout_scripts_stores_svelte_count };*/ Edit: That brings us back to the original naming conflict issue. If it were that easy, we could just leave the exported variable names as they were from the start. Edit 2: Maybe if we took this idea, but redefined var layout_global_html_svelte = create_ssr_component(($$result, $$props, $$bindings, slots) => {
...
let makeTitle = layout_scripts_make_title_svelte_makeTitle;
return `<html lang="${"en"}" class="${"svelte-196y134"}">${validate_component(Head, "Head").$$render($$result, { title: makeTitle(content.filename) }, {}, {})}
...
} |
It would be nice if we could omit the |
Simple string replacements can be problematic. Sometimes create_ssr_component(($$result, $$props, $$bindings, slots) => { and other times: create_ssr_component(($$result, $$props, $$bindings, $$slots) => { Should use regexp instead... |
It's not perfect, but this is slightly better now. Benefits from the new release:
Areas for improvement:
|
We're still relying on a quick fix during the SSR'ing of
cmd/build/client.go
to change variables into a component signature that can be reliably referenced by other components:plenti/cmd/build/client.go
Line 152 in 6312546
The problem is, we're assuming that everything with the original variable name and space on either side of it is a reference to that original variable. However, there could simply be other strings with the same text that shouldn't be replaced. A good example came to light when looking into stores. The variable name in the example is
count
but the title where it's displayed also contains the text "count." When the page hydrates, the client display looks fine:Hydrated page using JS
...but if JS is disabled (or still loading) it's not:
Static html page with JS disabled
We should use regex to make this replacement more precise and avoid these accidental string changes.
The text was updated successfully, but these errors were encountered: