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

Adding plugins to builds (DLLs) #8395

Closed
jodator opened this issue Nov 2, 2020 · 10 comments
Closed

Adding plugins to builds (DLLs) #8395

jodator opened this issue Nov 2, 2020 · 10 comments
Labels
domain:dx This issue reports a developer experience problem or possible improvement. domain:extending-builds Epic package:core package:engine support:2 An issue reported by a commercially licensed client. type:improvement This issue reports a possible enhancement of an existing feature.

Comments

@jodator
Copy link
Contributor

jodator commented Nov 2, 2020

The original story behind this issue can be read in #667. However, some things we discussed in the past may be obsolete today. Hence a new issue that will focus on research & implementation of allowing adding plugins to a compiled builds.


Key drivers to have this implemented:

  1. Ease of use by external developers.
    • In other words, it should be enable-able just by adding <script> tags of our special build and a bundled (or not) external feature.
  2. We should not lose tree-shaking ability in our builds but we're fine with bigger builds that allow adding plugins.
    • However, using the custom build, in that case, is OK.
  3. Base editor API should be de-duplicated in such builds.
    • At least enabled once - we can live with double code in that case.
  4. Public API should enable most of the integrations possible.
    • Only base editor API is needed, other things should work on top of that.
  5. CSS should work and not be duplicated.

We have talked about:

  • Explore possible directions:
    • Webpack's module federation API.
    • Webpack's DLL plugin.
    • Exposing CKEditor5 global object (available on window) and dot access to classes (base editor API).
      • it might require better naming for some classes/methods
  • Minimizing dependency on the engine, core, and (if possible) UI.
  • Deal with CSS de-duplication.
  • What modules should expose (as npm package)
    • for instance, every npm package can expose classes as exports in main.js file - this should work well with tree shaking.
    • minimal set of js and css file.
  • Conditional attach-to-global. The idea behind this is that feature attaches itself to a global object only if it is empty (so the first attach wins).
  • A minimal set of base features to expose is:
    • Engine
    • Core
    • Utils (because of EmitterMixin)
    • UI (might be tricky)
  • Resolve EmitterMixing dependency in the utils. Changing Plugin API might a bit might solve that. Check what we import from other features in the first place.
  • The main ckeditor5 package could be potentially used to expose the global API.
    • It should be possible to create a build with or without global object (tree shaking, optimizations, etc).

Related issues:


If you'd like to see this improvement implemented, add a 👍 reaction to this post.

@jodator jodator added domain:dx This issue reports a developer experience problem or possible improvement. package:core package:engine squad:dx type:improvement This issue reports a possible enhancement of an existing feature. Epic labels Nov 2, 2020
@jodator jodator self-assigned this Nov 2, 2020
@jodator
Copy link
Contributor Author

jodator commented Nov 2, 2020

Research summary:

@jodator
Copy link
Contributor Author

jodator commented Nov 2, 2020

#667 takeouts:

We should not rely on instanceOf checks in the editor features code. This causes problems when multiple imports occur. OTOH, we should not allow double-imports as only one engine plugin to ensure that all features operate on the same API version (hence, the dreaded ckeditor-duplicated-modules error.

Some ideas from the original issue, namely exposing editor Position, Range & conversion API (#4427, #4428), etc API was already implemented. Newer features are built-in this in mind so the importing from other packages is kept at a minimum... except ui package.

The idea to avoid importing from core (like Plugin or Command) was raised before but during the course of the years, we implemented "disabling API" for those, and we rely on basic plugin & command interface. (#667 (comment), #667 (comment))

Making CKEditor 5 as a peer dependency of all other packages - might be useful for exposing CKE5 global object (#1061, comments below: #667 (comment)).

CKEditor 5 global object (#667 (comment), #667 (comment), #667 (comment))

Duplicated CSS might be a bigger problem (#667 (comment) )

Previous proposal (#667 (comment))

  • exposing more API.
  • helper or utils (especially pure functions) shouldn't cause duplicated modules erros.
  • prevent loading plugin twice should be based on plugin name not the instance

CSS de-duplication ideas: #667 (comment).

@jodator
Copy link
Contributor Author

jodator commented Nov 3, 2020

#8402: Review cross-packages imports

  • Exposing API for cross-packages imports is not feasible ATM.
    • Some features are logical to export while others are used in one-two places as a utility method.
    • The work required to rewrite the features is too big and the ROI might be marginal.
    • We don't know what other developers import.
  • It might be worthwile to inpsect which modules or parts of code can't be duplicated and for which it doesn't matter.
    • The EmitterMixin problem.
    • The engine must be on the same level - might be feasible to reduce its usage outsdie.

@jodator
Copy link
Contributor Author

jodator commented Nov 16, 2020

#8397: It looks like we've missed the biggest premise of the Module Federation - to load code from external entry points at run time. This is a killer feature for micro-frontends but it uses an asynchronous-first approach. AFAICS this might not be feasible to use for sharing a code synchronously. Especially if we want to make it easier to use than we do currently.

@jodator
Copy link
Contributor Author

jodator commented Nov 24, 2020

#8398: The DLL is very promising and we will go further on this path. The main task for this will #8517 in which we will provide a bunch of DLLs and a way to use them together in a single HTML file as separate <script> tags + one file that ties them together. Something along the lines:

<script>
// Sudo import.
const {
ClassicEditor, HtmlEmbedEditing, HtmlEmbedUI
} = window.CKEditor5DLL( './src/index.js' );
const config = Object.assign( {}, ClassicEditor.defaultConfig, {
extraPlugins: [
HtmlEmbedEditing,
HtmlEmbedUI,
ComplexBox // exposed by the app.js
],
htmlEmbed: {
showPreviews: false
}
} );
config.toolbar.items.push( 'htmlEmbed' );
ClassicEditor.create( document.querySelector( '#editor' ), config )
.then( editor => {
window.editor = editor;
} );
</script>

The const { ClassicEditor, HtmlEmbedEditing, HtmlEmbedUI } = window.CKEditor5DLL( './src/index.js' ); should be resolved by the #8521 - implementing CKEditor 5 global.

@jodator
Copy link
Contributor Author

jodator commented Dec 3, 2020

After adding more packages to the base DLL and by providing the first DLL consumer plugins (BasicStyles & HtmlEmbed) we have a rather clear path to follow. You can read and track the progress here: #8517 (comment).

There are a few open questions, however, we don't see any clear blocker that would prevent us from delivering the whole thing using DLLs.

@jodator
Copy link
Contributor Author

jodator commented Dec 3, 2020

Due to the fact that we're in the middle of release and that the work on DLLs might take a bit of time I've introduced the dll-integration branch from current master. Until further notice, all PRs should be merged there.

@jodator
Copy link
Contributor Author

jodator commented Dec 4, 2020

The base DLL work is done. We've created a dll-integration with all the progress.

ATM you can checkout it and play with it:

  1. A dedicate sample was added.
  2. To build all DLL plugins and DLL consumers run yarn build:dll in the ckeditor5 main directory.
  3. After step 2 build the example DLL consumer plugin in packages/ckeditor5-dll/sample/plugin directory.
  4. Open the sample html page in the browser (might require http server).

This is an early build however and still many aspects of this must be polished.

@jodator
Copy link
Contributor Author

jodator commented Dec 8, 2020

We're implementing some improvements to the DLL build process. This,  unfortunately, means that the dll-integration branch will require using a linked package from @ckeditor/ckeditor5-dev-utils:

  1. Clone the git@github.com:ckeditor/ckeditor5-dev.git repository.
  2. Checkout the dll-integration branch.
  3. Run yarn install.
  4. Go to the packages/ckeditor5-dev-utils and link the package using yarn link.
  5. Go to ckeditor5 package and use linked package: yarn link @ckeditor/ckeditor5-dev-utils.

@Reinmar Reinmar changed the title Adding plugins to builds Adding plugins to builds (DLLs) Jan 12, 2021
@jodator jodator removed their assignment Jan 13, 2021
@Reinmar Reinmar modified the milestones: iteration 39, iteration 40 Jan 22, 2021
Reinmar added a commit that referenced this issue Feb 1, 2021
Other: Enabled creating builds that can be extended (with more plugins) without the need to being recompiled. This required splitting the project into the, so-called, DLL part and consumers of this DLL. Under the hood, the mechanism is based on [webpack DLLs](https://webpack.js.org/plugins/dll-plugin/). This is the first part of the required changes and it contains the necessary breaking changes (see the "MAJOR BREAKING CHANGES" section above). For more information see the ["DLL builds"](https://ckeditor.com/docs/ckeditor5/latest/builds/guides/development/dll-builds.html) guide. Closes [#8395](#8395).

Other: Introduced `Image.isImageWidget()` utility method.

Feature (core): Plugin collection will allow to require plugin by name when it is provided in `config.plugins` or if it was already loaded. Closes [#2907](#2907).

MAJOR BREAKING CHANGE: Several plugins are not anymore loaded automatically as dependencies of other plugins. From now on, they need to be provided to the editor creator manually (via `config.plugins`). This list includes:

- The `CloudServicesUploadAdapter` plugin no longer loads `CloudServices`. Make sure to add `CloudServices` to the editor plugins when using the  `CloudServicesUploadAdapter` or `EasyImage` features.
- The `EasyImage` plugin no longer loads `Image` and `ImageUpload`. Make sure to add `Image` and `ImageUpload` to the editor plugins when using the `EasyImage` feature.
- The `CKFinder` plugin no longer loads `CKFinderUploadAdapter`. The `CKFinderEditing` plugin no longer loads `ImageEditing` and `LinkEditing` features. Make sure to add `CKFinderUploadAdapter`, `Image`, and `Link` features to the editor plugins when using the `CKFinder` feature.
- The `Title` plugin no longer loads `Paragraph`. Make sure to add `Paragraph` to the editor plugins when using the `Title` feature.
- The `ListEditing` plugin no longer loads `Paragraph`. Make sure to add `Paragraph` to the editor plugins when using the `List` feature.
- The `LinkImageEditing` plugin no longer loads `ImageEditing`. Make sure to add `Image` to the editor plugins when using the `LinkImage` feature.
- The `LinkImageUI` plugin no longer loads `Image`. Make sure to add `Image` to the editor plugins when using the `LinkImage` feature.
- The `ExportPdf` plugin no longer loads `CloudServices`. Make sure to add `CloudServices` to the editor plugins when using the `ExportPdf` feature.
- The `ExportWord` plugin no longer loads `CloudServices`. Make sure to add `CloudServices` to the editor plugins when using the `ExportWord` feature.
pomek added a commit to ckeditor/ckeditor5-dev that referenced this issue Feb 1, 2021
Feature (utils): Added a module (`builds.getDllPluginWebpackConfig()`) that produces the webpack configuration for DLL. See ckeditor/ckeditor5#8395.
@martynawierzbicka martynawierzbicka added the support:2 An issue reported by a commercially licensed client. label Feb 19, 2021
@Reinmar
Copy link
Member

Reinmar commented Feb 23, 2021

There's still a long tail of followups, but we can consider DLLs ready to use. Docs will follow soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:dx This issue reports a developer experience problem or possible improvement. domain:extending-builds Epic package:core package:engine support:2 An issue reported by a commercially licensed client. type:improvement This issue reports a possible enhancement of an existing feature.
Projects
None yet
Development

No branches or pull requests

3 participants