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

DON'T MERGE - HiGlass Testing #42

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

DON'T MERGE - HiGlass Testing #42

wants to merge 3 commits into from

Conversation

alexkb0009
Copy link
Collaborator

@alexkb0009 alexkb0009 commented Aug 31, 2020

This branch is an attempt to have the higlass dependency and HiGlassPlainContainer component live in SPC (here) rather than the CGAP and 4DN repos, individually.

There is a roadblock to this currently in that HiGlass requires React-Bootstrap v0.32.4, while CGAP and 4DN are now on React-Bootstrap v1+. When HiGlass exists as dependency of either CGAP or 4DN, a copy of v0.32.4 is included along with it a sub-dependency and co-exists with v1+. However, when it is here, during development at least, since React-Bootstrap is a peer dependency, a symlink is created from SPC/node_modules/ to either 4DN or CGAP's React-Bootstrap library causing HiGlass to error as it isn't getting the right version of React-Bootstrap.

More research is needed on how to handle this issue. We could potentially wait for HiGlass to update to React-Boostrap v1+ - but then there is no guarantee the React-Bootstrap minor version used by HiGlass will always match the one used by CGAP and 4DN either and there might be issues due to this down the road as well.

For time being, HiGlass should live as a direct dependency of CGAP and 4DN repo (?).

More Thoughts

(please append more here or in comments if any)

  • It might be possible to install react-bootstrap@v1+ in this SPC repo as well, and import it into CGAP & 4DN -- but this really seems silly/roundabout/more-painful-to-have-to-do since would need to setup some file in SPC to import and export them. Would likely interfere with tree-shaking if not done well. Unlike w HiGlass where this approach makes much more sense to do because HiGlass depends on 4 dependencies (higlass/higlass-register/higlass-multivec/pixi.js) with a 'common logic wrapper' (HiGlassPlainContainer) that is ultimately imported, rather than higlass/etc dependencies directly imported themselves as with react-bootstrap.
  • Maybe can do some magic in package.json -- but so far no luck...
    • Maybe need to do intricate configuration magic in setup-npm-links-for-local-development.js also...
      • When I start using the word "magic" I lose a bit of confidence in what am talking about so think is good to defer to later or just not do..

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

Successfully merging this pull request may close these issues.

1 participant