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 @bazel/jasmine jasmineCore re-export so that jasmine-core transitive dep of jasmine transitive-dep is always used #697

Closed

Conversation

gregmagolan
Copy link
Collaborator

Ran into yet another corner case for re-exporting the correct jasmine-core from @bazel/jasmine npm package here: angular/angular#29913. I think I've thought it through fully now and this will always get the right version of jasmine-core

@gregmagolan
Copy link
Collaborator Author

For reference the case is. @bazel/jasmine dep on jasmine is hoisted to the root node_modules so its under node_modules/jasmine but root package.json has a conflicting dep on jasmine-core so its location is node_modules/jasmine/node_modules/jasmine-core.

const jasmineCore = require('jasmine-core');
const path = require('path');

// We want the jasmine-core version that is a transitive dependency of jasmine, however,
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm thinking, if we need jasmine-core here, should it be part of the dependencies of this package so we don't have to guess where it gets installed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ahhh.. Yes. That would be a much cleaner solution. @bazel/jasmine would have a dependency on the same version of jasmine-core that its jasmine dep does. Good call 😄

…ive dep of jasmine transitive-dep is always used
@gregmagolan
Copy link
Collaborator Author

Replaced by #698.

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.

3 participants