ckeditor / ckeditor4-presets

CKEditor 4 presets builder.
18 stars 27 forks source link

External plugins as node dependencies #43

Closed f1ames closed 3 years ago

f1ames commented 4 years ago

Since we did a first step and added exportpdf plugin as npm dependency maybe it will be good to use this approach for all external plugins we have here (and will have in the future)?

Now we are in the situation were some plugins are npm deps and some git submodules. Generally, it makes sense since exportpdf plugin is fetched from npm, while SCAYT and WSC are fetched from git repos.

However, it means we have two mechanisms for fetching external plugins. And since npm is now a step of the build proces itself it will mean better integration and prevent from forgetting about updating plugins submodules. IMHO npm deps are easier to diff and see which version is used - since submodules just point to commit hash so one have to click the link and see which particular tag/version it is, while git npm dep can link to specific branch, tag or commit which is clearly visible. WDYT?

jacekbogdanski commented 4 years ago

It would be nice to stick to one approach. Not sure how it works now under the hood, but makes also sense to clean up old implementation. I'm not sure if scayt and wsc are exposed as npm packages, so probably we would still need to use the repo link for that.

hub33k commented 4 years ago

Using plugins (scayt and wsc) as github dependencies is a little bit strange. These plugins don't have package.json so npm is throwing error (with yarn works well). So the one solution would be ask @WebSpellChecker for adding package.json to these plugins. WDYT?

"dependencies": {
  "ckeditor4-plugin-exportpdf": "1.0.0",
  "ckeditor-plugin-scayt": "https://github.com/WebSpellChecker/ckeditor-plugin-scayt",
  "ckeditor-plugin-wsc": "https://github.com/WebSpellChecker/ckeditor-plugin-wsc"
},
f1ames commented 4 years ago

Using plugins (scayt and wsc) as github dependencies is a little bit strange. These plugins don't have package.json so npm is throwing error (with yarn works well). So the one solution would be ask @WebSpellChecker for adding package.json to these plugins. WDYT?

:disappointed: Hmm, it seems so that without package.json it can be used like that... so we can ask WSC team to add package.json or try with some workaround like e.g. napa (https://www.npmjs.com/package/napa).

Could you check if napa package will solve our issue? :thinking:

f1ames commented 4 years ago

WebSpellChecker team just added packages to their repos so we can check how it works :tada:

https://github.com/WebSpellChecker/ckeditor-plugin-scayt/tree/release.5.7.0.0
https://github.com/WebSpellChecker/ckeditor-plugin-wsc/tree/release.5.7.0.0

CKEditorBot commented 3 years ago

Closed in https://github.com/ckeditor/ckeditor4-presets/pull/45