-
Notifications
You must be signed in to change notification settings - Fork 116
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
Bundle legend-graph separately from vscode-extension-dependencies #3859
Bundle legend-graph separately from vscode-extension-dependencies #3859
Conversation
🦋 Changeset detectedLatest commit: c02fe8e The changes in this PR will be included in the next version bump. This PR includes changesets to release 31 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 |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3859 +/- ##
==========================================
- Coverage 45.75% 45.71% -0.05%
==========================================
Files 2249 2249
Lines 392585 392409 -176
Branches 16334 17037 +703
==========================================
- Hits 179640 179392 -248
- Misses 212202 212312 +110
+ Partials 743 705 -38
|
@travisstebbins I understand the rationale of this change, but this is not solving the root problem, we have Now, in the particular setup we have, if you need to include some proprietary extensions, then the easiest will be to replicate exactly what we do here in |
Thanks for the tip! I do think in the longer term it would make sense to have CJS versions of all our packages available. However, in the meantime, I was able to get my proprietary extensions to work by creating a |
Summary
This PR bundles
@finos/legend-graph
separately from the rest of@finos/legend-vscode-extension-dependencies
so that when other VS Code extension packages have multiple dependencies that depend on@finos/legend-graph
, we can use webpack to alias all of them to the same instance of@finos/legend-graph
. This is necessary because without this, different dependencies will have their own copies of@finos/legend-graph
which will causeinstanceof
checks to fail, since they will technically be using different copies of the classes in@finos/legend-graph
.How did you test this change?
No testing as just adding rollup config to @finos/legend-graph.