fix: Correct template path resolution

[damassi]

User description

Taking the lib for a spin, I noticed that it wasn't resolving the paths to my preexisting templates via the register command. It would come out like:

supabase/migrations-templates/Users/cn/Sites/cn/my-project/supabase/migrations-templates/checkout.sql

And looking at the input paths this seemed indeed an issue:

path.join(baseDir, config.templateDir, templatePath)

so process.cwd() would append the dir and then the path.

Looking closer, by the time templatePath is input, its already been resolved here: https://github.com/t1mmen/srtd/blob/main/src/lib/templateManager.ts#L82

So this simplifies things and fixes the issue, allowing me to register my templates.

---

PR Type

Bug fix

---

Description

• Fixed incorrect template path resolution in registerTemplate.

• Removed redundant configuration-based path resolution logic.

• Simplified path resolution to use path.resolve.

---

Changes walkthrough 📝

	Relevant files
Bug fix	registerTemplate.ts Simplify and fix template path resolution logic                    src/utils/registerTemplate.ts Removed redundant getConfig import and usage. Simplified path resolution logic by removing unnecessary paths. Updated to use path.resolve for template path resolution. +1/-5

---

💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

[changeset-bot]

🦋 Changeset detected

Latest commit: 1f7d7f9

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@t1mmen/srtd	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[qodo-merge-pro-for-open-source]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Path Resolution The PR reduces the number of path resolution attempts to a single path. Consider keeping a fallback path resolution strategy to maintain flexibility. const pathsToTry = [ path.resolve(templatePath), ];

[qodo-merge-pro-for-open-source]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Score
Possible issue	✅ Enhance path resolution to handle both absolute and relative template paths Suggestion Impact: The commit directly implements the suggested path resolution changes by adding path.resolve(baseDir, templatePath) and path.join(baseDir, templatePath) to the pathsToTry array code diff: const pathsToTry = [ path.resolve(templatePath), path.resolve(baseDir, templatePath), - path.join(baseDir, templatePath) + path.join(baseDir, templatePath), ]; Add additional path resolution options to handle relative paths from baseDir. Currently, only absolute path resolution is attempted, which could fail for relative template paths. src/utils/registerTemplate.ts [10-12] const pathsToTry = [ path.resolve(templatePath), + path.resolve(baseDir, templatePath), + path.join(baseDir, templatePath) ]; Apply this suggestion Suggestion importance[1-10]: 8 Why: The suggestion addresses a significant limitation in the PR's path resolution logic. Adding support for both absolute and relative paths is crucial for proper template path handling, especially since the PR removed the previous template directory resolution.	8
General	Add input validation to prevent processing of invalid path parameters Add input validation to ensure templatePath is not empty or undefined before attempting path resolution. src/utils/registerTemplate.ts [9-12] export async function registerTemplate(templatePath: string, baseDir: string): Promise<void> { + if (!templatePath) { + throw new Error('Template path cannot be empty'); + } const pathsToTry = [ path.resolve(templatePath), ]; Apply this suggestion Suggestion importance[1-10]: 7 Why: Input validation is important for robust error handling and preventing potential runtime errors. The suggestion adds a necessary check that would fail fast with a clear error message rather than potentially causing issues during path resolution.	7

[t1mmen]

Ops, sorry about that. Thanks for fixing, @damassi 🙏

I'll take a closer look and ship this fix later today.

[t1mmen]

I made some improvements to path detection in general (so srtd can run from any directory in the project).

I had to move your changes into #main, to fix some biome/formatting issues.

Edit: Releases as 0.4.6, hopefully that works for you!

Thanks again for contributing, @damassi 🙏

[damassi]

Thank you for the quick fix! This tool is seriously the tool that SQL has needed for a long time. The possibilities are 🤯

[damassi]

Also @t1mmen - sorry for missing the formatting fixes; the issue I ran into requiring the --no-verify is that I don't use nvm to manage node versions, but rather asdf, so couldn't run the tool out of the box (even though I do use biome). So might want to make that contributor flow a bit more general.

[t1mmen]

This tool is seriously the tool that SQL has needed for a long time. The possibilities are 🤯

@damassi I'm slightly relieved this is the feedback I'm getting :D I sort of couldn't believe how ancient the DX around Postgres was in this day and age, compared to JS/TS-world. Surely I was missing something, I thought. Something like srtd had to exist, I figured... yet, I couldn't find it, hence this project :)

As for simplifying contributions, I'll probably re-visit that if issues/PR's keep coming. I wasn't really expecting anyone to notice this/care to fix things, but glad to be proven wrong :)

[damassi]

Yes, for example:

• All of our core evolving sql code now lives in src/sql -- with the rest of our source code!
• srtd watches src/sql/*, and inside there things are broken up into obvious files/categories one would expect
• folders such as sql/functions, sql/types, sql/rls, etc
• contributors now know exactly where to look for things (though obviously, one must still remain vigilant with postgres)

Would love to see this somehow integrated into the supabase cli itself 😉