Support for non-modular packages in tests and workspace resolver #2167
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
🦋 Changeset detectedLatest commit: a39a00c The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
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 |
cristiano-belloni
changed the title
added 2 commits
September 30, 2022 12:47
cristiano-belloni
requested review from
sgb-io,
ayseenver,
benpryke and
steveukx
sgb-io
reviewed
Oct 6, 2022
steveukx
reviewed
Oct 6, 2022
| @@ -153,7 +152,7 @@ export function analyzeWorkspaceDependencies( | |||
| // Exclude the root when analyzing package inter-dependencies | |||
| const packagesWithoutRoot = Array.from(workspacePackages.entries()).filter( | |||
| ([, pkg]) => { | |||
| return pkg.modular.type !== 'root'; | |||
| return pkg.modular?.type !== 'root'; | |||
There was a problem hiding this comment.
Minor thing, but this filter can be inlined to the next forEach on line 160 to avoid looping through the workspace twice.
As type has been added to the ModularWorkspacePackage, this can now be:
Suggested change
| return pkg.modular?.type !== 'root'; | |
| return pkg.type !== 'root'; |
steveukx
reviewed
Oct 6, 2022
| @@ -65,7 +65,8 @@ export async function resolveWorkspace( | |||
| const pkgPath = packageJsonPath(root); | |||
|
|
|||
| const json = await readPackageJson(isRoot, workingDirToUse, pkgPath); | |||
| const isModularRoot = json.modular?.type === 'root'; | |||
| const type = json.modular?.type || undefined; | |||
There was a problem hiding this comment.
Do we need to cater for converting to undefined any modular.type that has been explicitly set to null?
Suggested change
| const type = json.modular?.type || undefined; | |
| const type = json.modular?.type; |
|
Closing as it needs to be discussed further |
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.
This PR stems from the discussion here: #2168 and from the necessity of either:
a) not list non-modular workspaces in workspace-resolver or
b) list them, but clarifying that they are non-modular.
Since we don't want to impact how
mismatchedWorkspaceDependenciesworks and we like the idea of executing workspace commands on non-modular workspaces, this PR modifies the output of workspace-resolver, adding atypefield, and adds support for non-modular (from now: extraneous) workspaces inmodular test. In particular,modular testwill:--package,--changed,--ancestors), in addition to the usual modular packagesyarn workspace <extraneous-workspace-name> testfor each one of them, after running the regular jest testsI also refactored the
testcommand to factor out utilities and tasks.Note: The PR seems huge, but most of the content are new test fixtures to simulate a monorepo with examples of failing, successful and non-existent test scripts in extraneous workspaces, with regular Modular workspace ancestors.
TASKS
typeis first-classtestscript for non-modular workspaces or warn