Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reference to snippet and blocks #677

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

aswamy
Copy link
Contributor

@aswamy aswamy commented Dec 13, 2024

What are you adding in this PR?

Support references for snippets and blocks

  • Can't merge it yet because we are reading every file to check for references; should we preload them? memoize the references?

What's next? Any followup issues?

  • Support references when you select files in explorer
  • Support references found in schemas

Before you deploy

  • This PR includes a new checks or changes the configuration of a check
    • I included a minor bump changeset
    • It's in the allChecks array in src/checks/index.ts
    • I ran yarn build and committed the updated configuration files
      • If applicable, I've updated the theme-app-extension.yml config
  • I included a minor bump changeset
  • My feature is backward compatible
  • I included a patch bump changeset

Comment on lines +213 to +231
// TODO: Fix this; seems very costly
async function getLiquidFiles(uri: string) {
const rootUri = await findThemeRootURI(uri);

const blockFiles = await fs.readDirectory(path.join(rootUri, 'blocks'));
const sectionFiles = await fs.readDirectory(path.join(rootUri, 'sections'));
const snippetFiles = await fs.readDirectory(path.join(rootUri, 'snippets'));

const sources = [];
for(let [uri, _] of [...blockFiles, ...sectionFiles, ...snippetFiles]) {
const doc = documentManager.get(uri);
if (!doc || doc.type !== SourceCodeType.LiquidHtml) {
continue;
}
sources.push(doc);
}

return sources;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we keep a memoized ast for each file and invalidate it on file change? Reading every file when someone looks up references to the variable seems expensive.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant