Conversation
|
Needs to be brought up to date with mapping before merging. |
pimotte
left a comment
pimotte
left a comment
There was a problem hiding this comment.
I have one questions and once we resolve that I think this is ready for merge along with the companion PR:)
| "prosemirror-view": "1.34.3" | ||
| }, | ||
| "peerDependencies": { | ||
| "@codemirror/autocomplete": "6.18.1", |
There was a problem hiding this comment.
Why do we have these as peerDependencies? I understand that technically these might be optional, but in practice, mostly everyone who builds on Waterproof editor would need them, and it seems we could save some hassle by just including them.
There was a problem hiding this comment.
They are peerDependencies as we don't wont to bundle the code for codemirror here. If we would do that, then we get more than 1 version of @codemirror/state, in fact we get one per project that uses it (so every language package and waterproof-editor). This leads to the problem that you discovered in waterproof-vscode.
Using peerDependencies should mean that they get installed when waterproof-editor is installed as a dependency, since waterproof-editor is a library and not a 'final product' this was okay in my opinion.
Furthermore, having them separated in peerDependencies means that I can easily stop esbuild from bundling these, by refering to the package.json
2 participants
Targets the 'mapping' branch.