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

fix: check dependencyGraph root for version info #507

Conversation

BasKiers
Copy link
Contributor

@BasKiers BasKiers commented May 22, 2019

What did you implement:

fixes: #506

Version information of linked dependencies (e.g. link:../../packages/logger) could not be resolved resulting in the warning message "WARNING: Could not determine version of module XXX".

How did you implement it:

Add an additional fallback to retrieve dependency version from the root of the dependencyGraph.

When analysing the current fallback code, the current implementation will never work in my workspace since yarn list --depth=1 (called through packager.getProdDependencies(path.dirname(packageJsonPath), 1)) will never contain version information inside dependencyGraph.dependencies.${module.origin}.dependencies since that would require a depth greater than 1.

Note. I did add a eslint-disable-next-line comment so I was able to use an array as path to circumvent bugs when the package name contains a . (e.g. lodash.memoize)

How can we verify it:

The workspace for which this PR resolves this warning has the following (or similar) structure

backend/
    api/
        package.json
            `dependencies: { "@project/some-package": "link:../../packages/some-package" }`
packages/
    some-package/
        package.json
            `dependencies: { "other-package": "1.0.0" }`

Todos:

  • Write tests
  • Write documentation
  • Fix linting errors
  • Make sure code coverage hasn't dropped
  • Provide verification config / commands / resources
  • Enable "Allow edits from maintainers" for this PR
  • Update the messages below

Is this ready for review?: YES
Is it a breaking change?: NO

@BasKiers BasKiers force-pushed the fix/check-dependencygraph-version branch from c2ed28b to 8c61219 Compare May 22, 2019 10:52
@BasKiers BasKiers force-pushed the fix/check-dependencygraph-version branch from 8c61219 to 49fd5ec Compare May 22, 2019 11:15
@hassankhan
Copy link
Contributor

Hi @BasKiers, thanks for the PR! 👍 Any chance you could add a couple of tests, just to be on the safe side?

@miguel-a-calles-mba miguel-a-calles-mba added this to the 5.3.3 milestone May 13, 2020
@miguel-a-calles-mba miguel-a-calles-mba changed the base branch from master to release/5.3.3 May 13, 2020 14:39
@miguel-a-calles-mba miguel-a-calles-mba modified the milestones: 5.3.3, 5.4.0 Jun 11, 2020
@miguel-a-calles-mba miguel-a-calles-mba changed the base branch from release/5.3.3 to release/5.4.0 June 11, 2020 17:59
@j0k3r
Copy link
Member

j0k3r commented Jul 17, 2020

Closing in favor to #541

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

Successfully merging this pull request may close these issues.

Dependency versions of external modules cannot be resolved
4 participants