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

The editor doesn't work with zone.js (and hence Angular) #413

Closed
ssougnez opened this issue Mar 7, 2017 · 13 comments
Closed

The editor doesn't work with zone.js (and hence Angular) #413

ssougnez opened this issue Mar 7, 2017 · 13 comments
Labels
resolution:invalid This issue is invalid (e.g. reports a non-existent bug or a by-design behavior). status:discussion type:bug This issue reports a buggy (incorrect) behavior.

Comments

@ssougnez
Copy link

ssougnez commented Mar 7, 2017

Hi,

I'm currently trying to use ckeditor5 within an angular2 application and... I'm struggling a bit. I can create two different applications (one via the bare minimum for angular2 and the other via the bare minimum for ckeditor5), but when I merge them, it does not work anymore.

First of, here are the main pieces of the application. Let's start with the webpack configuration file:

var path = require('path')
var webpack = require('webpack');
var HtmlWebpackPlugin = require('html-webpack-plugin');
var ExtractTextPlugin = require('extract-text-webpack-plugin');

module.exports = {
  context: __dirname,
  target: 'web',
  
  entry: {
    'polyfills': './src/polyfills.ts',
    'vendor': './src/vendor.ts',
    'app': './src/main.ts'
  },

  output: {
    path: './dist',
    publicPath: 'http://localhost:8080/',
    filename: '[name].js',
    chunkFilename: '[id].chunk.js'
  },

  resolve: {
    extensions: ['.ts', '.js'],
    modules: [
      path.resolve( __dirname, 'packages' ),
			'node_modules'
		]
  },

  module: {
    rules: [
      {
        test: /\.ts$/,
        loaders: [
          {
            loader: 'awesome-typescript-loader',
            options: { configFileName: './tsconfig.json' }
          } , 'angular2-template-loader'
        ]
      },
      {
        test: /\.html$/,
        loader: 'html-loader'
      },
      {
        test: /\.(png|jpe?g|gif|svg|woff|woff2|ttf|eot|ico)$/,
        loader: 'file-loader?name=assets/[name].[hash].[ext]'
      },
      {
        test: /\.css$/,
        exclude: './src/app',
        loader: ExtractTextPlugin.extract({ fallbackLoader: 'style-loader', loader: 'css-loader?sourceMap' })
      },
      {
        test: /\.css$/,
        include: 'src./app',
        loader: 'raw-loader'
      },
      {
          test: /\.js$/,
          include: path.resolve(__dirname, "node_modules/@ckeditor"),
          use: [{
              loader: 'babel-loader',
              options: {
                  presets: ['es2015-webpack2'],
                  cacheDirectory: true
              }
          }]
      },      
      {
        test: /\/ckeditor5-.*\.svg$/,
        use: [ 'raw-loader' ]
      },
      {
        test: /\.scss$/,
        use: [
          'style-loader',
          {
            loader: 'css-loader',
            options: {
              minimize: true
            }
          },
          'sass-loader'
          ]
        }
    ]
  },

  plugins: [
    // Workaround for angular/angular#11580
    new webpack.ContextReplacementPlugin(
      // The (\\|\/) piece accounts for path separators in *nix and Windows
      /angular(\\|\/)core(\\|\/)(esm(\\|\/)src|src)(\\|\/)linker/,
      './src', // location of your src
      {} // a map of your routes
    ),

    new webpack.optimize.CommonsChunkPlugin({
      name: ['app', 'vendor', 'polyfills']
    }),

    new HtmlWebpackPlugin({
      template: './src/index.html'
    }),

    new ExtractTextPlugin('[name].css')
  ]
};

This configuration is a merge between https://github.com/CKEditor5/ckeditor5.github.io/blob/master/webpack.config.js and https://angular.io/docs/ts/latest/guide/webpack.html. However, I moved the reference to "regenerator-runtime/runtime" in a vendor.ts file:

import '@angular/platform-browser';
import '@angular/platform-browser-dynamic';
import '@angular/core';
import '@angular/common';
import '@angular/http';
import '@angular/router';
import 'rxjs';

import 'regenerator-runtime/runtime';

import '@ckeditor/ckeditor5-editor-classic/src/classic';

import '@ckeditor/ckeditor5-enter/src/enter';
import '@ckeditor/ckeditor5-typing/src/typing';
import '@ckeditor/ckeditor5-paragraph/src/paragraph';
import '@ckeditor/ckeditor5-undo/src/undo';
import '@ckeditor/ckeditor5-basic-styles/src/bold';
import '@ckeditor/ckeditor5-basic-styles/src/italic';
import '@ckeditor/ckeditor5-image/src/image';

For the rest, my component is very simple and is composed of the ts file:

import { Component } from '@angular/core';

import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classic';

import Enter from '@ckeditor/ckeditor5-enter/src/enter';
import Typing from '@ckeditor/ckeditor5-typing/src/typing';
import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
import Undo from '@ckeditor/ckeditor5-undo/src/undo';
import Bold from '@ckeditor/ckeditor5-basic-styles/src/bold';
import Italic from '@ckeditor/ckeditor5-basic-styles/src/italic';
import Image from '@ckeditor/ckeditor5-image/src/image';

@Component({
  selector: 'my-app',
  templateUrl: './app.component.html'
})
export class AppComponent 
{ 
  ngOnInit()
  {
    let a = ClassicEditor.create(document.getElementById("editor"), {
        plugins: [ Enter, Typing, Paragraph, Undo, Bold, Italic, Image ],
        toolbar: [ 'bold', 'italic', 'undo', 'redo' ]
    }).then((a: any) => {;

        console.log(a)
    });
  }
}

And the template:

<div id="editor">
  <p></p>
</div>

The result of this is the following error:

EXCEPTION: Uncaught (in promise): TypeError: emitter.on is not a function
TypeError: emitter.on is not a function
    at FocusTracker.listenTo (http://localhost:8080/vendor.js:7819:11)
    at FocusTracker.listenTo (http://localhost:8080/vendor.js:47087:75)
    at FocusTracker.add (http://localhost:8080/vendor.js:94255:9)
    at new ClassicEditorUI (http://localhost:8080/vendor.js:118738:21)
    at new ClassicEditor (http://localhost:8080/vendor.js:67544:14)
    at http://localhost:8080/vendor.js:67602:18

The stack trace is actually way longer than that because of zonejs. As I realise that it is pretty difficult to help me like this, here is the code of this app : ng2.zip. Just run npm run build and then start a simple http server in the "dist" folder.

@Reinmar Reinmar mentioned this issue Mar 7, 2017
@ssougnez
Copy link
Author

ssougnez commented Mar 7, 2017

Just discovered that the problem is probably not related to Angular but Zone.js. Instead of trying to implement ckeditor in an angular2 app, I try the other way around. So, I started from the working minimum working project of ckeditor5 with webpack and I just added this package:

"zone.js": "^0.7.4"

Then, I added the following line in my main file:

require('zone.js/dist/zone');

Then I get the error:

Unhandled Promise rejection: e.on is not a function ; Zone: ; Task: Promise.then ; Value: TypeError: e.on is not a function

The message is slightly different but I think that the root cause is the same, so it seems that the incompatibility is with Zone and not Angular. I will try to dig a bit deeper but I'm pretty sure that this exceeds my skills :p

EDIT

After some searches, it looks like the "issue" (between quotes because I'm really not sure it really is an issue ^^) comes from the function listenTo in "emittermixin.js:139". Actually, just to test, I replaced the last line by:

if (emitter.on)
{
	// Finally register the callback to the event.
	emitter.on( event, callback, options );
}

And the editor suddenly showed up, although it does not work correctly (the bold function does not work, hitting Ctrl + B put the caret at the beginning, etc...).

I also put a:

console.log(emitter);

Just before this condition and everytime the next line works (so the condition), emitter contains an object, for example:

ButtonView {locale: Locale, _viewCollections: Collection, _unboundChildren: ViewCollection, _bindTemplate: Object, template: Template…}

However, when it fails, it does not contain an object as such but an HTML element, for example:

<div class="ck-blurred ck-editor__editable ck-editor__editable_inline" contenteditable="true" role="textbox" aria-label="Rich Text Editor, main"></div>

So it would look like the issue comes from the fact that an event is bound to an HTML element (that maybe is not yet on the page? Or in the shadow DOM?).

Once again, I'm far from an expert, so maybe the issue is not in ckeditor but in Zone. I leave you, experts, the call :p

EDIT II - He's back

Hi, me again :-D I'm still investigating and I ended up in the function isDomNode from "emittermixin.js:283" and I replaced the line:

return node && isNative( node.addEventListener );

By

return node && node.addEventListener;

And poof, everything works... Of course, I changed the behaviour of the method, this was just a test, but I'll check what isNative is doing in order to see if there is an issue or not. I'll be back... again :-D

@ssougnez
Copy link
Author

ssougnez commented Mar 7, 2017

Hi, here is the conclusion of my search (the final chapter of the lovely little story above). I'm not 100% sure but if I'm not wrong, ZoneJs overrides some function like setTimeout, addEventListener, etc... in order to execute these functions in a specific context (the whole goal of the library).

Therefore, when isNative checks if the method is native or not, I gets "false" because this method is not anymore the native one but the one overriden by ZoneJs. I could verify that by typing this in the "Google Developer Toolbar":

document.getElementById('editor').addEventListener

This returned me:

function addEventListener(){return f(this, arguments)}

While doing this in a simple HTML page (without ZoneJS) returned me:

function addEventListener() { [native code] }

Off the top of my head, the solution would consist in having a white list of all method overriden by ZoneJs and checking this first in isNative but it seems pretty dirty to me, it would be nice to find a way to know if the function has been overriden by ZoneJs or not.

I'll have a look at that tomorrow, altough, I'm wondering... Is it up to ckeditor to be compatible with ZoneJs or the other way around? I feel that it's the former, but I'm not 100% sure... On step further would be to know if lodash should be compatible with ZoneJs or not (as I just saw that isNative is a method of "lodash"). Any idea?

@Reinmar
Copy link
Member

Reinmar commented Mar 8, 2017

First of all – the award of the hero of the month goes to you! 🥇 🍾 You saved us a lot of time and trouble! Huge thanks!

So, I don't like what zone.js did. It's a bad practice to modify native prototypes – it's asking for troubles. We'd never do that and I think no library should touch objects it doesn't own. Especially... if it's such a low-level library like zone.js because then the impact is so wide and unpredictable that it just gives me creeps ;/

Anyway, there are two options – either zone.js fixes this or we do. From a puristic POV I'd say that it should be zone.js. It's very unlikely that they'll stop changing the native prototypes, so at least they could fix the toString() properties of functions they assign to native prototypes. This would fix the root of the issue and prevent similar errors in the future (we're using Lodash's isNative() function, so there may be more libraries doing so).

However, we could also consider changing our current "is native node" check. I like the current one, but we could figure something more generic (like you proposed). After all, we would never name any of our methods "addEventListener" :D.

So, I'd propose first to ask zone.js's team to fix it on their side and wait for a reply.

Thank you again for looking into this.

@Reinmar Reinmar changed the title Mixing ckeditor5 and angular2 make ckeditor fails The editor doesn't work with zone.js (and hence Angular) Mar 8, 2017
@Reinmar Reinmar added status:discussion type:bug This issue reports a buggy (incorrect) behavior. labels Mar 8, 2017
@ssougnez
Copy link
Author

ssougnez commented Mar 8, 2017

Alright, I'll create an issue in the zone repo and I'll keep you up to date ;-)

@ssougnez
Copy link
Author

Hi, just good a feedback and they will make isNative return true for all methods which were patched by zone.js, so nothing to do for you guys ^^

@Reinmar
Copy link
Member

Reinmar commented Mar 10, 2017

Yeah, I've seen it – angular/zone.js#666. And it's a perfect issue number for such a bug :D

Thanks!

@ssougnez
Copy link
Author

Thought exactly the same ;-)

I'll follow the release of zone.js and then I'll come back here to post if they really solved the issue or not. I guess you can close this issue.

Thanks

@ssougnez
Copy link
Author

This issue can be closed as the version 0.8.4 of zone.js solves it.

Thanks ;-)

@Reinmar Reinmar added the resolution:invalid This issue is invalid (e.g. reports a non-existent bug or a by-design behavior). label Mar 29, 2017
@pburrows
Copy link

pburrows commented Nov 7, 2017

FWIW, I've updated to the latest zone.js and still get this error.

@Reinmar
Copy link
Member

Reinmar commented Nov 13, 2017

This is sad ;/ Could you check whether this is a problem with isNative() again?

@pburrows
Copy link

It's now about two weeks later, I never went back to look at this issue, but it is magically working. shrug

@Reinmar
Copy link
Member

Reinmar commented Nov 22, 2017

Better this than when things magically break :D. Thanks for the info!

@oliverguenther
Copy link

Just noting here for anyone stumbling upon this that is indeed not fixed even with the current zone.js version 0.8.20. Almost commented to suggest to get rid of the isNative check, then stumbled upon @Reinmars comment #768 (comment) which resolves the issue 👏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
resolution:invalid This issue is invalid (e.g. reports a non-existent bug or a by-design behavior). status:discussion type:bug This issue reports a buggy (incorrect) behavior.
Projects
None yet
Development

No branches or pull requests

4 participants