Skip to content

Commit

Permalink
♻️ Refactor variable access in compiled template code
Browse files Browse the repository at this point in the history 
- Previously, template variables were accessed directly using property access (e.g., `{{ my_value }}` was converted to something like `renderer.print(vars.my_value)`).
- This approach proved to be difficult to maintain, so it has been refactored to use function-based access instead.
- Now, template variables are accessed using a `get()` method (e.g., `renderer.print(vars.get('my_value'))`).
- This change should improve the overall maintainability and flexibility of the templating engine.
  • Loading branch information
@skerit
skerit committed Apr 28, 2024
1 parent 69ff2ca commit 1d63520
Show file tree
Hide file tree
Showing 11 changed files with 299 additions and 164 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
* Add `Variables#get(key)` & `Variables#setFromTemplate(key, value)` method
* Don't let `Hawkejs.doNextSync(promise)` swallow errors
* Use methods for getting (and proxy traps for setting) variables on the `Variables` class
* Refactor variable access in compiled template code

## 2.3.19 (2024-04-13)

Expand Down
66 changes: 35 additions & 31 deletions lib/core/renderer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -519,15 +519,15 @@ Renderer.setProperty(function theme() {
*
* @author Jelle De Loecker <[email protected]>
* @since 2.2.15
* @version 2.2.15
* @version 2.4.0
*
* @type {HTMLElement}
*/
Renderer.setProperty(function current_element() {

if (this.on_template_root_level) {
if (this.current_variables.$ancestor_element) {
return this.current_variables.$ancestor_element;
if (this.current_variables.has('$ancestor_element')) {
return this.current_variables.get('$ancestor_element');
}

return;
Expand Down Expand Up @@ -2247,7 +2247,7 @@ Renderer.setMethod(function applyElementOptions(element, options, for_sync_rende

for (i = 0; i < options.variables.length; i++) {
val = options.variables[i];
element[Hawkejs.VARIABLES][val.name] = val.value;
element[Hawkejs.VARIABLES].set(val.name, val.value);
}
}

Expand Down Expand Up @@ -2675,29 +2675,33 @@ Renderer.setMethod(function hasBeenRegistered(element) {

let variables = this.root_renderer.variables;

if (!variables || !variables.__he_elements) {
if (!variables || !variables.has('__he_elements')) {
return false;
}

return variables.__he_elements.indexOf(element) > -1;
return variables.get('__he_elements').indexOf(element) > -1;
});

/**
* Register an element that needs to be dried
*
* @author Jelle De Loecker <[email protected]>
* @since 2.0.0
* @version 2.1.3
* @version 2.4.0
*
* @param {Element} element
*/
Renderer.setMethod(function registerElementInstance(element) {

var variables = this.root_renderer.variables,
let he_elements,
variables = this.root_renderer.variables,
id = element.hawkejs_id || element.dataset.hid;

if (!variables.__he_elements) {
variables.__he_elements = [];
if (!variables.has('__he_elements')) {
he_elements = [];
variables.set('__he_elements', he_elements);
} else {
he_elements = variables.get('__he_elements');
}

if (!id) {
Expand All @@ -2706,12 +2710,12 @@ Renderer.setMethod(function registerElementInstance(element) {
// Set the id
element.setIdentifier(id);
} else {
if (variables.__he_elements.indexOf(element) > -1) {
if (he_elements.indexOf(element) > -1) {
return;
}
}

variables.__he_elements.push(element);
he_elements.push(element);
});

/**
Expand Down Expand Up @@ -3041,7 +3045,7 @@ Renderer.setCommand(function bottom() {
*
* @author Jelle De Loecker <[email protected]>
* @since 1.0.0
* @version 2.3.16
* @version 2.4.0
*
* @param {Array|String} names The partial to render
* @param {Object} options
Expand Down Expand Up @@ -3078,7 +3082,7 @@ Renderer.setMethod(function addSubtemplate(names, options, variables) {

// The Templates render method expects fully prepared variables.
variables = this.prepareVariables(variables);
variables.$ancestor_element = ancestor_element;
variables.setFromTemplate('$ancestor_element', ancestor_element);

templates.render(variables).done(function done(err, block) {

Expand Down Expand Up @@ -3651,7 +3655,7 @@ Renderer.setMethod(function createOpenElement(name, add_identifier) {
*
* @author Jelle De Loecker <[email protected]>
* @since 2.2.3
* @version 2.3.11
* @version 2.4.0
*
* @param {HTMLElement} element
*/
Expand All @@ -3662,7 +3666,7 @@ Renderer.setMethod(function addFallbackParentElementGetter(element) {
}

// See if an ancestor element has been defined
let $ancestor_element = this.current_variables.$ancestor_element;
let $ancestor_element = this.current_variables.get('$ancestor_element');

if (!$ancestor_element) {
return;
Expand Down Expand Up @@ -3992,7 +3996,7 @@ Renderer.setMethod(function add_link(href, options) {
*
* @author Jelle De Loecker <[email protected]>
* @since 1.0.0
* @version 2.2.16
* @version 2.4.0
*
* @param {String} name
* @param {Mixed} value
Expand All @@ -4001,19 +4005,19 @@ Renderer.setCommand(function set(name, value) {

if (arguments.length == 1) {

if (this.current_template && this.current_template.variables && typeof this.current_template.variables[name] != 'undefined') {
return this.current_template.variables[name];
if (this.current_template && this.current_template.variables && this.current_template.variables.has(name)) {
return this.current_template.variables.get(name);
}

// If there is a parent renderer, look there if this does not have it
if (!this.is_root_renderer && typeof this.variables[name] == 'undefined') {
return this.root_renderer.set(name);
if (!this.is_root_renderer && this.variables.has(name)) {
return this.root_renderer.get(name);
}

return this.variables[name];
return this.variables.get(name);
}

this.variables[name] = value;
this.variables.set(name, value);
});

/**
Expand Down Expand Up @@ -4082,7 +4086,7 @@ Renderer.setCommand(function internal(name, value) {
*
* @author Jelle De Loecker <[email protected]>
* @since 1.0.0
* @version 2.0.0
* @version 2.4.0
*
* @param {String} name
* @param {Mixed} value
Expand All @@ -4092,8 +4096,8 @@ Renderer.setCommand(function expose(name, value) {
if (arguments.length == 1) {

// Look inside the viewrender first
if (typeof this.expose_to_scene[name] !== 'undefined') {
return this.expose_to_scene[name];
if (this.expose_to_scene.has(name)) {
return this.expose_to_scene.get(name);
}

// If the item hasn't been found yet, look in the scene
Expand All @@ -4108,15 +4112,15 @@ Renderer.setCommand(function expose(name, value) {
return;
}

this.expose_to_scene[name] = value;
this.expose_to_scene.set(name, value);
});

/**
* Set this variable to be used on the server side only
*
* @author Jelle De Loecker <[email protected]>
* @since 1.1.1
* @version 2.2.16
* @version 2.4.0
*
* @param {String} name
* @param {Mixed} value
Expand All @@ -4126,14 +4130,14 @@ Renderer.setCommand(function serverVar(name, value) {
if (arguments.length == 1) {

// If there is a parent renderer, look there if this does not have it
if (!this.is_root_renderer && typeof this.server_variables[name] == 'undefined') {
if (!this.is_root_renderer && this.server_variables.has(name)) {
return this.root_renderer.serverVar(name);
}

return this.server_variables[name];
return this.server_variables.get(name);
}

this.server_variables[name] = value;
this.server_variables.set(name, value);
});

/**
Expand Down
Loading

0 comments on commit 1d63520

Please sign in to comment.