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

Getting error "Uncaught SyntaxError: Identifier '_typeof' has already been declared" #405

Closed
3Cbwaltz opened this issue Dec 22, 2015 · 26 comments

Comments

@3Cbwaltz
Copy link

Just tried to set up a basic react component using alloy editor. I am using browserify, from reading the other comments, I tried to only require alloy editor in the componentDidMount() function. When I do this I get "Uncaught SyntaxError: Identifier '_typeof' has already been declared" in the Chrome console.

Here is my component:

var React = require('react');
var ReactDOM = require('react-dom');
var _ = require('lodash');
var $ = require('jquery');

var MyComponent = React.createClass({
   componentDidMount(){
      var AlloyEditor = require('alloyeditor');
      AlloyEditor.editable(this.refs.content);
   },
   render() {
     return(<div ref="content">foo</div>);
   },
});

module.exports = MyComponent;

Any thoughts?

@ipeychev
Copy link
Contributor

You may look here for an React component. It uses the old React however, you will have to update the code in the gist.

Hope that helps,

@ipeychev
Copy link
Contributor

Some piece of advice from me, since I'm seeing this over and over again:
it is much better to not include third party scripts in the final bundle of your application, including AlloyEditor.

AlloyEditor is an third party script for your app, so there is no sense to use Browserify in order to include it. This will just add unnecessary code to your application and if you have one bundle for your application, as most people have, this will be nothing but big performance penalty for your application! The script will be huge, you will probably have to specify window.ALLOYEDITOR_BASEPATH, every time you change your application AlloyEditor's file will be downloaded among with the other scripts of your app, despite you will probably update your app much often than AlloyEditor.
Keeping the above in mind, I would consider to include AlloyEditor (and all other third party scripts) separately from the application's code.

You may take a look on the gist here. If you update it to use the latest version of React, 0.14.3 for example (AlloyEditor > 0.7 will not work with React 0.13.x), and change a bit the gist, you will be able to render the component on the server, which will render just the content, but in the browser will create an instance of the editor.

Hope that helps!

Thanks,

@dantman
Copy link
Contributor

dantman commented Dec 23, 2015

@ipeychev requiring 3rd party libraries is half the point of browserify. It would be foolish to do development without 3rd party libs or re-invent loading of 3rd party libraries when require was designed for that. Going back to arcane use of globals and explicit script tags is - when the use of modules and transpilers is growing and growing and even the language itself is standardizing these new things.

If an application changes infrequently and AlloyEditor is almost guaranteed to be used. Then there is nothing wrong with bundling it with the code.

If separating 3rd party and internal code or code used on only certain pages for performance is desired. Then that's what --require/--external, factor-bundle, and potentially other transforms are for. No need to drop all the advantages browserify gives you just for that.

Likewise if dynamically loading an infrequently used 3rd party module for performance is desired there are still ways of doing that within the browserify ecosystm. You can use one of the many async loaders to load a bundle made with --require/--external, factor-bundle. Try dynapack. And other methods of dynamic loading bundles only require someone to write a transform for them.

@ipeychev
Copy link
Contributor

@dantman, I didn't say anything about using a build tool but separating the third party libraries from application's files. In this case - sure, use any kind of build tool you want. I am giving a warning against bundling AlloyEditor among with the other files, and this is in general valid for most of the other 3rd party libraries.

dantman:
If an application changes infrequently and AlloyEditor is almost guaranteed to be used. Then there is nothing wrong with bundling it with the code.

In most of the cases this will be just performance penalty. Even if not changing frequently, it will be much better to separate the scripts (including using a tool) and add AlloyEditor on the bottom of the page or to the head but mark the script as async, to not block the initial rendering.

We switched to more or less performance related discussion, but just one more thing - I recently saw an application, which was bundling its own scripts among with about 10 different 3rd party libraries, including huge ones, like Cesium. Because, well, why not?
The first thing I did was to drop this approach and we gained about 20% (in some browsers 30%) less loading time, not to mention the caching.

Hope that helps,

@beeant
Copy link

beeant commented Dec 25, 2015

I'm also having this issue with alloyeditor v0.7.2 installed from npm

SyntaxError: Identifier '_typeof' has already been declared

@ipeychev
Copy link
Contributor

Wait please a while, I will publish an example, today or tomorrow.

@ipeychev
Copy link
Contributor

Here is a repository with an example.

Hope that helps!

Thanks,

@beeant
Copy link

beeant commented Dec 26, 2015

Thank you for the example.

I took a brief look at the repository, and I noticed that you separate AlloyEditor, React and ReactDOM into vendor.bundle.js in webpack.

I did the same, and webpack successfully compiled both client.js (app) and vendor.bundle.js. (It was failing before due to the _typeof error.)

screenshot from 2015-12-26 18-29-43

but the error starts to appear on the Developer's tool's console.

screenshot from 2015-12-26 18-29-02

@ipeychev
Copy link
Contributor

Did you update AlloyEditor to the latest version?

@ipeychev
Copy link
Contributor

alloy-editor-core.js is being loaded. This file contains no CKEdtor's engine, and no React. This is useful in other cases - when there is CKEditor already on the page and you want to render on the server.
Probably you didn't update to the latest version or you are explicitly loading this file.

Thanks,

@beeant
Copy link

beeant commented Dec 26, 2015

I just updated AlloyEditor to from 0.7.2 to 0.7.5, and the error is gone!
Thank you!

@beeant
Copy link

beeant commented Dec 26, 2015

PS: The vendor.bundle.js separation is actually pretty good to speed up the site loading (assuming the browser will load both files in parallel)

@ipeychev
Copy link
Contributor

Glad it worked! Yeah, separation makes much more sense than putting everything in one file.

@beeant
Copy link

beeant commented Dec 26, 2015

I got another error:
screenshot from 2015-12-26 20-59-45

After taking a look at your example repository again, I see that you have a gulp task to copy AlloyEditor directory to dist.
And also, you set a value for window.ALLOYEDITOR_BASEPATH.

PS: I think this is an unusual setup for a module. I'm currently hesitating if I should follow this setup. Like I'm not using gulp for my project that will use AlloyEditor, if I want to proceed to use AlloyEditor, I will have to install gulp just for this purpose.

@ipeychev
Copy link
Contributor

The error with the MIME type is not related to AlloyEditor. If this file is present, something on the server should be tweaked.

Gulp is not necessary. I'm using gulp to copy AlloyEditor to the dist folder, you may use whatever you want or to copy it manually.
Setting window.ALLOYEDITOR_BASEPATH and window.CKEDITOR_BASEPATH is required with this setup in order to retrieve the corresponding lang file, etc.
In the "normal" setup - with just adding the script on the page it is not.

I hope you realize how complex and advanced this setup is. At least I'm now aware of any other editor on Earth, able to work in this way.

@beeant
Copy link

beeant commented Dec 26, 2015

Yes, I see that the setup is complex, but I'm not sure if this is the most ideal or advanced setup. As I prefer a module to be simple to integrate by just doing var AlloyEditor = require("alloyeditor"); or import AlloyEditor from "alloyeditor" and let webpack remove the unused code during minimize.

I will setup gulp now and follow your way for now.

Thank You!

@ipeychev
Copy link
Contributor

Yeah, this would be the only step, if there wasn't the problem with language files. Normally, you would want your users to get automatically AlloyEditor translated on their language, wouldn't you?
That's why we have to detect the language and load it dynamically, otherwise we should add all the 60+ languages and pollute the network with garbage.
This is the reason for copying AlloyEditor to the dist folder. Actually, you can avoid the copying - just set window.ALLOYEDITOR_BASEPATH and window.CKEDITOR_BASEPATH to point to AlloyEditor in node_modules and it should work. In my example I'm copying it because I prefer to serve everything from one folder.

I will try to see if we can provide the path via some property instead to add it to window. This will be a good improvement.

@beeant
Copy link

beeant commented Dec 26, 2015

Oh yes, the language files can be a problem, I'm not sure but there should be a better solution instead of setting a global BASEPATH variable. I think popular module like momentjs has multi-language support, I haven't seen how they do it though.

Pointing it to node_modules might probably make it even worse. I just go with the copy all the dist files solution for now (although I still feel uneasy after doing it).

Yes, I would prefer setting the path from options object properties when initiating AlloyEditor. Like the properties can set the BASEPATH, theme, cfk plugins path.

I finished setting up gulp, webpack, and webpack-dev-server for my project, and AlloyEditor works great! Just there are some other few things I need to find out, like binding the text value to React state/props, and also installing CFK plugins for photo upload after drag&drop.

@ipeychev
Copy link
Contributor

My idea was to set basePath like this:

var AlloyEditor = require('alloyeditor');
AlloyEditor.basePath = 'alloyeditor/';

This would work perfectly for AlloyEditor, but unfortunately there is no way to set the basePath for CKEditor's engine :(
It is just not designed to work in this way, so for now the global variables will be the only way.

Thanks,

@rexpan
Copy link

rexpan commented Jan 18, 2016

Hi @ipeychev
I still got this issue.
It seems the _typeof function has been defined at 3 different places.
image

@ipeychev
Copy link
Contributor

You get this error on our demo page? How weird...
Which version is this browser? Maybe you are using some extension, which for some reason breaks the editor? Why don't you try to disable them entirely and try again, if it works, it should be easy to verify which is the guilty one.
I cannot reproduce this, so we should take a look on your environment.

@rexpan
Copy link

rexpan commented Jan 18, 2016

@ipeychev Yeah, you are right.

It only occurred on Chrome Canary, I tried at Chrome stable and it works fine.

My Chrome Canary Version 49.0.2623.0 canary (64-bit) running on Windows 10 with some flags were turned on (like Enable Experimental JavaScript).

Edit: I just disabled Experimental JavaScript and it still happends.

@ipeychev
Copy link
Contributor

Great! There is nothing to do then, if it stops working on the released Chrome, then we will get worried :)

@kushal
Copy link
Contributor

kushal commented Jan 25, 2016

Well, it's true that _typeof is defined 3 times in the file, is there any easy way to prevent that?

@ipeychev
Copy link
Contributor

We don't care at all about Chrome Canary, but in any case I just run Canary to see what's going on. The version is:
Version 50.0.2630.0 canary (64-bit), running on Mac.
And there are no any errors in the console, see the attached screenshot. I also have
Experimental JavaScript Mac, Windows, Linux, Chrome OS, Android enabled.

screen shot 2016-01-25 at 10 19 33 pm

@rexpan
Copy link

rexpan commented Jan 26, 2016

I try this again in Chrome Canary Version 50.0.2630.1 canary SyzyASan.
It has been fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants