feat: add EJS template support with handling of dynamic content #453
Conversation
There was a problem hiding this comment.
Hello @HarshDodiya1 thank you for taking time and implementing EJS template support. I left a few comments to better understand your code, please have a look 👇
Also note that we are using prettier in the project, please use it to format your code before committing.
PS: I merged main branch into your feature branch and resolved one conflict related to missing createNewLineLoader() that was deleted in the meantime.
| if (normalizedText.includes(EJS_PLACEHOLDER_PREFIX)) { | ||
| // Store original EJS expression | ||
| const placeholder = placeholders.find(p => normalizedText.includes(p.id)); | ||
| if (placeholder) { | ||
| result[getPath(node)] = placeholder.content; | ||
| } |
There was a problem hiding this comment.
Why are you removing EJS tags (line 33) and then adding them back here? I found the generated code is the same if I removed both parts. Did I miss anything?
Same in push() method below.
| <html lang="es"> | ||
| <body> | ||
| <h1>Bienvenido, <%- user.name %>!</h1> | ||
| <p>Tu saldo: <%= balance %></p> | ||
| </body> | ||
| </html> | ||
| `.trim().replace(/\s+/g, ' ').replace(/>\s+</g, '><'); |
There was a problem hiding this comment.
I think this is slightly confusing, as it is not obvious what the expected output is. Can you include it as expected in the template literal here?
There was a problem hiding this comment.
I also noticed the tests are failing - the strings seem correct, just whitespaces seem off.
There was a problem hiding this comment.
I agree with Matej here, 100%
What kind of change does this PR introduce?
Feature - Adds EJS (Embedded JavaScript) template support to Lingo.dev's localization system.
Issue Number
Fixes [#299]
Summary
Other Information
Added new loader:
packages/cli/src/cli/loaders/ejs.tsExtended test suite:
index.spec.tsUpdated formats:
packages/spec/src/formats.tsReference: EJS Documentation