Feat/undefined cases #168
Merged
Feat/undefined cases #168
Conversation
This file contains hidden or 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
- Modified LaunchQLProject.parseProjectTarget to handle undefined targets in workspace context - Updated deploy/revert/verify methods to process ALL modules when target is undefined - Added comprehensive test suite to verify undefined target functionality works correctly - When target is undefined and recursive=true, operations apply to ALL modules in workspace - When target is undefined and recursive=false, operations use current module context Co-Authored-By: Dan Lynch <[email protected]>
- Simplified (!target || extension === name) to (extension === name) - If target is undefined, toChange is also undefined, making !target check redundant - Applied fix to deploy, revert, and verify methods Co-Authored-By: Dan Lynch <[email protected]>
- Simplified (extension === name) ? to extension === name ? - Applied to all three methods: deploy, revert, verify Co-Authored-By: Dan Lynch <[email protected]>
- Add getAllWorkspaceExtensions() method to eliminate code duplication - Replace moduleNames[0] approach with null-based workspace-wide operations - Update parseProjectTarget() to return null for workspace context - Handle null name case properly in deploy/revert/verify methods - Add error handling for non-recursive operations on workspace Co-Authored-By: Dan Lynch <[email protected]>
When name is null (workspace-wide operation), there's no specific target change to apply, so logic should be extension === name ? toChange : undefined Co-Authored-By: Dan Lynch <[email protected]>
- Rename apps/index to _virtual/app in synthetic root pattern - Fix getAllWorkspaceExtensions to directly collect extensions from all modules - Add comprehensive test coverage for undefined target scenarios - Ensure workspace-wide operations work correctly with undefined targets - Maintain existing interface for backward compatibility Co-Authored-By: Dan Lynch <[email protected]>
- Replace broken getAllWorkspaceExtensionsWithDependencyOrder with working implementation - Use extDeps with virtual module for proper workspace-wide dependency ordering - Rename method to getWorkspaceExtensionsInDependencyOrder for clarity - Ensure revert operations use proper reverse dependency ordering - Fix double-reversal issue in revert method - Fix bug where external extensions were incorrectly processed as workspace modules Co-Authored-By: Dan Lynch <[email protected]>
Co-Authored-By: Dan Lynch <[email protected]>
- Created comprehensive test file for getWorkspaceExtensionsInDependencyOrder method - Added snapshot tests using existing fixtures (simple-w-tags, launchql, simple) - Tests verify dependency ordering properties and basic functionality - Made getWorkspaceExtensionsInDependencyOrder method public for testing - All 6 tests pass with proper snapshot generation Co-Authored-By: Dan Lynch <[email protected]>
- Use Set-based deduplication to prevent duplicate extensions in resolved and external arrays - Preserve dependency ordering while eliminating duplicates - Update snapshot tests to reflect the corrected behavior - Maintain performance by using Sets for O(1) lookup during deduplication Co-Authored-By: Dan Lynch <[email protected]>
- Update function definition in deps.ts from extDeps to resolveExtensionDependencies - Update all imports and function calls in launchql.ts to use new name - Fix test file dependency-resolution-basic.test.ts to use new function name - Maintain all existing functionality while improving code clarity - All tests pass with the new naming convention Co-Authored-By: Dan Lynch <[email protected]>
…lveExtensionDependencies - Add comparison test demonstrating equivalence between current and simplified approaches - Replace complex 30+ line implementation with clean 8-line version - Maintain all functionality while removing unnecessary Sets, loops, and manual extension collection - Test results show outputs are identical for most fixtures with only minor ordering differences - Updated snapshots to reflect simplified implementation Co-Authored-By: Dan Lynch <[email protected]>
Co-Authored-By: Dan Lynch <[email protected]>
Co-Authored-By: Dan Lynch <[email protected]>
Co-Authored-By: Dan Lynch <[email protected]>
- Clarifies that method returns extensions, not a module - Explains the algorithm and use case for revert operations - Documents why it finds the 'top' dependent module - Addresses misleading method name with detailed explanation Co-Authored-By: Dan Lynch <[email protected]>
- Comprehensive analysis of findTopDependentModule() usage and problems - Documents single call site in revert() method - Identifies inconsistencies with deploy/verify patterns - Recommends removal in favor of consistent dependency resolution - Provides detailed impact analysis and code change suggestions Co-Authored-By: Dan Lynch <[email protected]>
…resolution Co-Authored-By: Dan Lynch <[email protected]>
- Remove double reversal in workspace-wide revert operations (name === null) - Implement consistent reversal at execution time for all revert scenarios - Add comprehensive analysis in REVERSE_REVERT.md documenting the issue and solution - Ensure proper reverse dependency order for safe revert operations Co-Authored-By: Dan Lynch <[email protected]>
- Remove unnecessary toChange special case in revert method - Use module-specific extensions when name is specified (consistent with verify) - Only use workspace-wide resolution when name === null - Aligns revert behavior with deploy and verify methods Co-Authored-By: Dan Lynch <[email protected]>
…pendency resolution - Restore workspace-wide extension resolution when reverting specific changes - Ensures dependent modules are included for proper dependency ordering - Fixes 'Cannot revert table_products: required by my-third:create_schema' error - Both failing tests now pass: deployment-scenarios.test.ts and forked-deployment-scenarios.test.ts Co-Authored-By: Dan Lynch <[email protected]>
- Explain why workspace-wide resolution is necessary for database safety - Document the dependency chain problem with concrete examples - Clarify difference between revert and verify operations - Address user feedback about the toChange logic Co-Authored-By: Dan Lynch <[email protected]>
- Implement workspace-wide extension resolution for undefined targets - Apply proper dependency handling in toChange scenarios - Add comprehensive test coverage for revert truncation scenarios - Verify proper stopping behavior in various revert operations - Maintain database safety while improving efficiency - Update documentation with truncation optimization explanation Co-Authored-By: Dan Lynch <[email protected]>
- Correct example to show [my-third, my-second] when reverting to my-second - Clarify that dependent modules must be reverted before their dependencies - Address backwards logic in truncation explanation Co-Authored-By: Dan Lynch <[email protected]>
- Implement truncateExtensionsToTarget method to truncate workspace extensions - Apply truncation before reversal in toChange scenarios - Add comprehensive test coverage for truncation scenarios - Verify proper stopping behavior in various revert operations - Maintain database safety while improving efficiency - Update documentation with truncation optimization explanation Co-Authored-By: Dan Lynch <[email protected]>
Co-Authored-By: Dan Lynch <[email protected]>
- Always use workspace-wide resolution for revert operations to prevent database constraint violations - Ensures dependent modules are reverted before their dependencies - Fixes CI failure: 'Cannot revert create_another_table: required by my-third:create_table' - Resolves LaunchQLError in revert-truncation-scenarios.test.ts Co-Authored-By: Dan Lynch <[email protected]>
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.
No description provided.