Elderjs / elderjs

Elder.js is an opinionated static site generator and web framework for Svelte built with SEO in mind.
https://elderguide.com/tech/elderjs/
MIT License
2.11k stars 53 forks source link

Fix parsing hydrate-options={{ loading: 'eager', preload: true }} #249

Closed dmitrysmagin closed 2 years ago

dmitrysmagin commented 2 years ago

It was mentioned here in multiple issues that parsing of hydrate-options={{}} is broken in 1.7.5

https://github.com/Elderjs/elderjs/issues/227 https://github.com/Elderjs/elderjs/issues/226

This is because hydrate-options={{ loading: "eager", preload: true }} is transformed into '{ loading: "eager", preload: true }' which is then passed into JSON.parse(), so:

JSON.parse('{ loading: "eager", preload: true }') <--- won't work JSON.parse('{ "loading": "eager", "preload": true }') <--- works fine JSON.parse("{ 'loading': 'eager', 'preload': true }") <--- won't work ;) json uses double quotes

Ideally we would have to use some kind of parser to transform object style into json style, but there's a much simpler solution:

--- a/node_modules/@elderjs/elderjs/build/partialHydration/inlineSvelteComponent.js
+++ b/node_modules/@elderjs/elderjs/build/partialHydration/inlineSvelteComponent.js
@@ -15,7 +15,15 @@ function escapeHtml(text) {
 }
 exports.escapeHtml = escapeHtml;
 function inlinePreprocessedSvelteComponent({ name = '', props = {}, options = '', }) {
-    const hydrationOptions = options.length > 0 ? { ...defaultHydrationOptions, ...JSON.parse(options) } : defaultHydrationOptions;
+    const parseOptions = () => {
+        try {
+            return new Function(`return ${options}`)();
+        } catch(e) {
+            console.error(`Error in hydrate-options for ${name}`);
+            return {};
+        }
+    };
+    const hydrationOptions = options.length > 0 ? { ...defaultHydrationOptions, ...parseOptions() } : defaultHydrationOptions;
     const hydrationOptionsString = JSON.stringify(hydrationOptions);
     const replacementAttrs = {
         class: '"ejs-component"',

This is a patch for the compiled inlineSvelteComponent.js, but the idea is applicable for https://github.com/Elderjs/elderjs/blob/master/src/partialHydration/inlineSvelteComponent.ts as well.

The only downside is that the options are not computable, we can't write hydrate-options={{ loading: someVar, preload: !!(anotherBool && thirdBool) }} because options are parsed literally as-is and are not pre-calculated

dmitrysmagin commented 2 years ago

No reaction? Patch seems to be reliable.

eight04 commented 2 years ago

There is a better solution i.e. stop using JSON.parse on hydrate-options: https://github.com/Elderjs/elderjs/blob/162972ccdbad7f5569096bd5794cd59271c6004a/src/partialHydration/inlineSvelteComponent.ts#L29

nickreese commented 2 years ago

I’ve got a kiddo in the hospital for the next 7-10 days. @eight04 if you want to merge I can release when I get back but honestly this just hasn’t been a prio at the moment.

eight04 commented 2 years ago

@nickreese I can fix some bugs on master but it may conflict with feat/typescript branch. Is that OK?

nickreese commented 2 years ago

@eight04 yep go for it. Typescript’s internal reload needs work and isn’t ready for production.

eight04 commented 2 years ago

@dmitrysmagin Try if #257 can solve the issue. You can install elderjs from that branch by running the following command in your project:

npm install Elderjs/elderjs#fix/hydrate-options
dmitrysmagin commented 2 years ago

Great, thanks!