Introducing UndefinedObject check
#115
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
charlespwd
reviewed
Sep 27, 2023
packages/theme-check-common/src/checks/undefined-object/index.ts
Outdated
Show resolved
Hide resolved
charlespwd
reviewed
Sep 27, 2023
packages/theme-check-common/src/checks/undefined-object/index.ts
Outdated
Show resolved
Hide resolved
charlespwd
reviewed
Sep 27, 2023
charlespwd
reviewed
Sep 27, 2023
Co-authored-by: CP Clermont <cp.clermont@shopify.com>
- Eliminate unnecessary usage of `node.blockEndPosition` since the `assign` tag doesn't have a corresponding end tag - Implement `docset->objects[0]->access` logic to consider only global objects during this check
|
Thanks a lot for the review, @charlespwd! Changes applied :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes https://github.com/Shopify/theme-check-js/issues/66
--demo.mp4
What are you adding in this PR?
The
UndefinedObjectcheck is being introduced to identify references to undefined Liquid objects.This implementation largely mirrors the Ruby version, and intentionally skips snippet assets. This is because these assets depend on variables defined in parent templates, and Theme Check is designed to focus on single asset checks.
Looking ahead, the following issue is planned to be addressed next: #114. Once resolved, developers will be able to annotate available variables, thereby establishing an interface/contract between Liquid templates.
Finally, this check doesn't currently provide a fix or suggestion, as the main approaches would be about (1) removing the variable or (2) ignoring the check. To enhance this, I've opened a new issue: #113. This proposes a global 'ignore check' implementation, so all checks will be able to suggest an ignore dialog. I don't foresee this leading to overuse of the ignore function, as I the ignore-code introduces some noise.
Before you deploy
changesetto this PR