fix: resolve hardcoded logic entry point in TemplateArchiveProcessor#108
fix: resolve hardcoded logic entry point in TemplateArchiveProcessor#108Sup-ri-tha wants to merge 2 commits intoaccordproject:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Resolves #107 by removing the hardcoded logic/logic.ts lookup in TemplateArchiveProcessor and instead selecting a logic entry point from the compiled script set so templates with different logic filenames (and non-TS files in logic/) don’t crash.
Changes:
- Add runtime logic entry point discovery for both
trigger()andinit()to replacecompiledCode['logic/logic.ts']. - Add a clearer failure mode when no entry point is found.
- Update
package-lock.json(appears unrelated to the functional change).
Reviewed changes
Copilot reviewed 1 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| src/TemplateArchiveProcessor.ts | Replaces hardcoded logic module path with a discovered entry point for evaluation in trigger() and init(). |
| package-lock.json | Removes multiple "peer": true fields from various dependency entries. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
src/TemplateArchiveProcessor.ts
Outdated
| const entryPoint = Object.keys(compiledCode).find( | ||
| key => key.startsWith('logic/') && | ||
| !key.includes('generated/') && | ||
| key.endsWith('.ts') && | ||
| compiledCode[key].code !== undefined |
There was a problem hiding this comment.
Done. Added a path depth check to restrict matches to direct children of logic/ only. It prevents nested files from being picked up accidentally.
src/TemplateArchiveProcessor.ts
Outdated
| key => key.startsWith('logic/') && | ||
| !key.includes('generated/') && | ||
| key.endsWith('.ts') && | ||
| compiledCode[key].code !== undefined |
There was a problem hiding this comment.
Skipped this. noErrorValidation: true is already set in the compiler options and the errors array isn't reliably populated in all cases. Felt like it could cause more problems than it solves. Let me know if you would still like this in.
src/TemplateArchiveProcessor.ts
Outdated
| const entryPoint = Object.keys(compiledCode).find( | ||
| key => key.startsWith('logic/') && | ||
| !key.includes('generated/') && | ||
| key.endsWith('.ts') && | ||
| compiledCode[key].code !== undefined | ||
| ); | ||
| if(!entryPoint) { | ||
| throw new Error('Could not find compiled logic entry point'); | ||
| } |
There was a problem hiding this comment.
Done. Extracted it into a shared private helper method, called from both trigger() and init().
src/TemplateArchiveProcessor.ts
Outdated
| const entryPoint = Object.keys(compiledCode).find( | ||
| key => key.startsWith('logic/') && | ||
| !key.includes('generated/') && | ||
| key.endsWith('.ts') && | ||
| compiledCode[key].code !== undefined | ||
| ); | ||
| if(!entryPoint) { | ||
| throw new Error('Could not find compiled logic entry point'); | ||
| } | ||
| const evaluator = new JavaScriptEvaluator(); | ||
| const evalResponse = await evaluator.evalDangerously( { | ||
| templateLogic: true, | ||
| verbose: false, | ||
| functionName: 'trigger', | ||
| code: compiledCode['logic/logic.ts'].code, // TODO DCS - how to find the code to run? | ||
| code: compiledCode[entryPoint].code, |
There was a problem hiding this comment.
Added 3 unit tests covering: non-standard filenames, exclusion of generated and non-ts files, and the missing entry point case. Initially tried full end-to-end tests but hit a pre-existing twoslash limitation where the compiler crashes on a second invocation in the same Jest process. Unit tests felt like the cleaner solution here.
src/TemplateArchiveProcessor.ts
Outdated
| const entryPoint = Object.keys(compiledCode).find( | ||
| key => key.startsWith('logic/') && | ||
| !key.includes('generated/') && | ||
| key.endsWith('.ts') && | ||
| compiledCode[key].code !== undefined | ||
| ); | ||
| if(!entryPoint) { | ||
| throw new Error('Could not find compiled logic entry point'); |
There was a problem hiding this comment.
Skipped this as well. Listing internal file paths in errors felt like it could confuse template authors more than help. Happy to revisit if you think it's worth adding!
Both trigger() and init() methods hardcoded 'logic/logic.ts' as the logic entry point when looking up compiled code. This caused a crash if the logic file had any other name. Dynamically find the entry point by searching for a .ts file directly in the logic/ folder that is not auto-generated and has compiled output. Fixes accordproject#107 Signed-off-by: Supritha Rajashekar <supritharajashekar10@gmail.com>
Signed-off-by: Supritha Rajashekar <supritharajashekar10@gmail.com>
Sup-ri-tha
force-pushed
the
Sup-ri-tha/i107/fix-hardcoded-logic-entrypoint
branch
from
23bf01c to
09597be
Compare
Summary
Fixes #107 by dynamically resolving the logic entry point in
TemplateArchiveProcessor, eliminating the hardcodedlogic/logic.tsassumption noted by the// TODO DCScomment.Problem
When
trigger()andinit()look up compiled code, they hardcodecompiledCode['logic/logic.ts'].code. This fails in two ways - if the logic file has any other name,compiledCode['logic/logic.ts']returnsundefinedcausing a crash. Additionally,compiledCodecontains non-TypeScript files likelogic/README.mdwhose invalid compiled output causesbtoa()inJavaScriptEvaluatorto throwSyntaxError: Invalid or unexpected token.Solution
Added a dynamic entry point lookup that searches for a
.tsfile directly inlogic/that is not auto-generated and has valid compiled output, replacing the hardcoded string in both methods.Changes
src/TemplateArchiveProcessor.ts: Replace hardcodedlogic/logic.tswith dynamic entry point lookup in bothtrigger()andinit()Testing
logic/logic.tsas entry point for existing test archiveChecklist