partridgejiang / Kekule.js

A Javascript cheminformatics toolkit.
http://partridgejiang.github.io/Kekule.js
MIT License
248 stars 61 forks source link

Using Kekule npm module with Webpack does not load Kekule properly. #36

Open AaronLlanos opened 6 years ago

AaronLlanos commented 6 years ago

References: #32 Steps to duplicate:

Begin any project with webpack base.

yarn add kekule

In a js file you import from index.html:

import {Kekule} from 'kekule'
or 
const {Kekule} = require('kekule')

console.log(Kekule)

Now run webpack to minify your project and open the output file and receive:

kekule.js:504 Uncaught TypeError: Cannot read property 'getElementsByTagName' of undefined
    at analysisEntranceScriptSrc (kekule.js:504)
    at init (kekule.js:631)
    at kekule.js:671
    at Object.<anonymous> (kekule.js:673)
    at Object.defineProperty.value (page.js:128749)
    at __webpack_require__ (bootstrap 2cce55343c358056e4dd:19)
    at Object.Array.indexOf.i (kekule.composer.component.js:4)
    at __webpack_require__ (bootstrap 2cce55343c358056e4dd:19)
    at Object.<anonymous> (moldraw.js:6)
    at __webpack_require__ (bootstrap 2cce55343c358056e4dd:19)
    at webpackContext (.*\.js$:111)
    at sl.module.factory.js:11

In this particular case, we see that document is undefined. In #32 we see that passing the window object works for the webpack compiler but when the script is trying to load in the browser we get errors that it cannot find the proper files. This is because webpack did not see explicit requires of those files and loses them in the build process.

Potential Fixes:

  1. Instead of appending to a script tag to them DOM, check if webpack was used and instead require the scripts.

    • Cons:
    • Each file must now officially export the class it's building since webpack compilation for the browser does not have access to vm.runInThisContext.
    • Each file imported to kekule.js must now be imported and exported appropriately for webpack and node
  2. Somehow figure out how to import a context of a module. Basically to do for webpack what vm.runInThisContext does for node.

    • Cons:
    • It doesn't exist (to my knowledge)
AaronLlanos commented 6 years ago

Updated the PR to contain steps for potential fixes

partridgejiang commented 6 years ago

Hi @AaronLlanos, I myself have tested for several days but still haven't found an elegant solution. However, the JavaScript files inside kekule/dist directory have already been compressed (with kekule.min.js, which packed all common modules in one file), and it is not quite necessary to pack them with webpack again. Maybe webpack.IgnorePlugin can be used in project to explicitly bypass all files in Kekule directory?

AaronLlanos commented 6 years ago

Webpack itself does not break. It will actually bundle all of the necessary files, but the problem is that when dist/kekule.js or src/kekule.js loads in the browser with webpack, it can't actually write the script tags to the DOM because it's already bundled through the build process. As a resul, I think that if we were to update the individual files in src to contain module.exports = <exported_Class_here> to the files, it would be a really easy way to remove the vm.runInContext solution.

Doing so does two things for us. First, it would make this compatible with more node environments. Second, it also opens up compatibility for running this lib more friendly with major frameworks that build with webpack. i.e. React, Angular, Vue, etc...

gywgithub commented 6 years ago

I'm having the same problem, and kekule can't add to the vue app. A nice js library, but I can't use it in projects.

AaronLlanos commented 6 years ago

@gywgithub I have a webpack abled fork here: https://github.com/AaronLlanos/Kekule.js/tree/webpack-kekule I currently use this in a React project of mine and it works with the latest frontend libs such as webpack, react, vue, etc..

partridgejiang commented 6 years ago

@AaronLlanos Great work, thanks, :).

gywgithub commented 6 years ago

@AaronLlanos Well done, Thank you very much.

GabiAxel commented 6 years ago

Any chance to merge @AaronLlanos fork? Maybe have two variants of what is now kekule.js/kekule.min.js so that it could be used both ways? Awesome as the fork is, when attempting to use it, I found out it was missing the properties from 3e1001837dcdea17461b4ab33519c12551fa2939 , and generally there is always a risk of it being not up to date with this repository, so it would be really great if they were merged.

AaronLlanos commented 5 years ago

My fork still uses the kekule.min.js but it utilizes webpack to generate out the minified file. My fork also makes assumptions about inchi, indigo, and openbabel that are not supported of the way @partridgejiang I'm sure would like to go.

GabiAxel commented 5 years ago

Alright. I found sort of a workaround for my requirements. Thanks all for your work on this.

partridgejiang commented 5 years ago

@AaronLlanos Yes of course. Would you please pull the fork to this repository? Thanks a lot.

robertf224 commented 5 years ago

@AaronLlanos would love this in here as well

robertf224 commented 5 years ago

@AaronLlanos have been playing around with your fork, mostly working but noticing I get Uncaught TypeError: this.getIsotopeId is not a function when I try to add periodic elements to my drawings. Any idea what's up there? Is there a particular branch/commit I should use (currently using latest commit of webpack-kekule branch)?

Also apologies for following up on this issue but your fork doesn't have an issues tab :(

Update: got it working on moldraw-master :)

AaronLlanos commented 5 years ago

@robertf224 My apologies for my lack of participation here. I figured I'd still post the info just in case anyone else is wondering. The workflow I am using and will keep as a convention in my forked repo is:

mpomet commented 5 years ago

I tryed to use Kekule in an angular app and didn't manage to make it work.

Starting with https://github.com/AaronLlanos/Kekule.js/

master is not working

import { Kekule } from 'kekule';
console.log(Kekule); // undefined

kekule-master seems to work but style are not loaded
The dist/ folder only contains kekule.min.js.
Themes, styles and extra are not loaded and do not exists in dist/ folder.
Also we cannot load styles from node_modules/kekule/demos/libs/kekule/themes/default/kekule.css as 'kekule' is the module name, so the following error occurs:
Module not found: SyntaxError: node_modules/kekule/demos/libs/kekule/package.json (directory description file): SyntaxError: Unexpected token } in JSON at position 395

Is there any solution to load and use Kekule in an angular application ? Is there a particular commit I have to reference to make it work ?

Thank you

[EDIT] I also found in dist/kekule.webpack.dev.js the following :

require("../src/widgets/themes/default/kekule.css");

But cannot use it too as this file does not exists

partridgejiang commented 5 years ago

@mpomet Just try using the master branch of https://github.com/partridgejiang/Kekule.js? In our own test, the following code works well with webpack and the style sheet can be properly loaded:

const Kekule = require('kekule');
console.log(Kekule);

By the way, some special function parameters should not be mangled in the compression process of webpack, for exmaple, in the webpack.config.js:

optimization: {
        minimizer: [
            new UglifyJsPlugin({
                uglifyOptions: {
                    mangle: {
                        reserved: ['$super', '$origin']
                    },
                },
            }),
        ],
    }
mpomet commented 5 years ago

@partridgejiang thank you, I manage to get it works with master indeed.

For anyone trying to use this lib with angular (v2 to 8+) that's what i did:

// import { Kekule } from 'kekule'; doesn't work
// const Kekule = require('kekule'); doesn't work neither
// import * as Kekule from 'kekule'; // that works but some errors happens while running

// typings.d.ts
declare var Kekule: any; // Global typing works

// Now we can use kekule
const viewer = new Kekule.ChemWidget.Viewer(...);

Then you have to load styles (angular.json):

      "architect": {
        "build": {
          "options": {
            ...
            "styles": [
              ...
              "./node_modules/kekule/dist/themes/default/kekule.css"
            ],
robotoer commented 4 years ago

Unfortunately with the current master (Dec 26, 2019):

    "kekule": "github:partridgejiang/Kekule.js#afd3b0a1ee78dfd38ed61f404e24295303fe9c6a",

importing using all three methods described by @mpomet above produce the following error for me:

kekule.min.js:1 Uncaught TypeError: Cannot read property 'createElement' of undefined
    at kekule.min.js:1
    at Object../node_modules/kekule/dist/kekule.min.js (kekule.min.js:1)
    at __webpack_require__ (bootstrap:79)
    at Object../node_modules/kekule/dist/kekule.webpack.prod.js (kekule.webpack.prod.js:2)
    at __webpack_require__ (bootstrap:79)
    at kekule.js:909
    at Object.<anonymous> (kekule.js:914)
    at Object../node_modules/kekule/dist/kekule.js (vendor.js:273567)
    at __webpack_require__ (bootstrap:79)
    ...

This error seems to be coming from the loading of the kekule webpack dist file.

This wasn't happening with (Oct 12, 2019):

    "kekule": "github:partridgejiang/Kekule.js#6ca9cd5fa75a634e1916e3e67e86411d32483627",
mpomet commented 4 years ago

@robotoer I've updated my last answer as it wasn't correct.
For me it works this way (tested with angular 8)

amitsetty commented 4 years ago

So I'm not the most experience programmer but I've spent the majority of a couple days dealing with this and would appreciate any tips and assistance

Currently trying to find a working solution on React and I get this error when using the fork

I'm only getting this issue when in production, when I'm in my development build everything is working smoothly

Uncaught TypeError: t is not a function
    at e.initialize (kekule.min.js:1)
    at new e (kekule.min.js:1)
    at Function.getInstance (kekule.min.js:1)
    at e.exports (kekule.min.js:1)
    at Object.<anonymous> (kekule.min.js:1)
    at t (kekule.min.js:1)
    at kekule.min.js:1
    at Object.<anonymous> (kekule.min.js:1)
    at a ((index):1)
    at Module.302 (main.379a58fc.chunk.js:1)
    at a ((index):1)
    at Object.169 (main.379a58fc.chunk.js:1)
    at a ((index):1)
    at f ((index):1)
    at Array.e [as push] ((index):1)
    at main.379a58fc.chunk.js:1

I've tried countless other things and would be happy to post the errors they give me here.

Tried to use the typescript solution abovedeclare var Kekule: any however in React I can't figure out where I can declare this variable and how to update the modules to hold it. Could anyone help me get towards any working solution? Thanks

partridgejiang commented 4 years ago

@amitsetty, could any demo files be provided here? Otherwise the problem may not be easily figured out from the error stack.

amitsetty commented 4 years ago

@partridgejiang Of course, here's a demo in react with all the files https://github.com/amitsetty/demo-kekule-react

There's also a branch with the webpack'd version Both versions work in dev mode (they're just console.logging the module) but don't work in production (there's an error)

Feel free to see the error and all in action by just cloning and running npm run build -> serve -s build

I can also demo anything else you'd like just let me know what will help you.

Thanks for your help so much - you're saving my project :)

partridgejiang commented 4 years ago

@amitsetty, the problem is caused by the js minification process in build. You have to manually set the some mangle configs for webpack. In the node_modules/react-scripts/config/webpack.config.js, please add a line of code in after line 234:

optimization: {
  ...
  minimizer: [
    ...
    new TerserPlugin({
      ...
      mangle: {
        safari10: true,   // line 234
        reserved: ['$super', '$origin']    // add this line of code
      }
      ...
    })
    ...
  ]
  ...
}

After that, the build should be OK.

Another approach is to use react-app-rewired to overwrite this configs rather than directly modifying it. Please refer to https://github.com/timarney/react-app-rewired.

amitsetty commented 4 years ago

@partridgejiang thank you so much for your help. I just implemented this and it works great however I think it increased the loading time of the website by about 10s. Does this make sense? Is there any way around it?

partridgejiang commented 4 years ago

@amitsetty The change of webpack.config.js just tells webpack not to mangle some special function parameters to a short variable name such as e and does nothing else. So in my consideration the loading time should not be slowed down so much. In my own test your demo file can be loaded and parsed in less than 700ms in my computer.

amitsetty commented 4 years ago

Hey there @partridgejiang , so far this has been working great. We've been using the molecule editor and now tried to incorporate a 3D molecule viewer. Is this supported in React? I seem to be getting an error that It seems that your web browser is not modern enough to support the drawing function. Please update it.

Feel free to check out my setup , I reproduced the error here https://github.com/amitsetty/render-3d-kekule-test

partridgejiang commented 4 years ago

Hi @amitsetty, the 3D rendering functions of Kekule.js requires a thrid-party library: Three.js. You have to import it in your code, something like:

import * as THREE from 'three';  // supposing the Three.js has been installed by npm
window.THREE = THREE;  // A trick, make Kekule.js know the exists of Three.js
const Kekule = require('kekule');  // use require instead of import here, since import can not be after a normal statement
// ...

Some discussions at issue https://github.com/partridgejiang/Kekule.js/issues/33#issuecomment-594470323_ and https://github.com/partridgejiang/Kekule.js/issues/161#issue-575481424 may also be helpful.

amitsetty commented 4 years ago

Great, that's somewhat working now. However, the molecule being rendered is displaying in the 3D viewer but as a flat molecule - without being 3d. Any idea how i go about fixing this? Thanks

partridgejiang commented 4 years ago

@amitsetty, it depends on the coordinates of the molecule source file. Most molecule files only include 2D coordinates of atoms illustrating the basic constitutions. Kekule.js does not have the ability to calculate out the 3D conformations from 2D coordinates (it is fairly a complex or time consuming job). In other words , files with real 3D coordinates should be provided to 3D viewer to display a proper 3D model. Here are some 3D source file examples: https://partridgejiang.github.io/Kekule.js/demos/chemFiles/3D/methane.mol https://partridgejiang.github.io/Kekule.js/demos/chemFiles/3D/cyclohexane.mol https://partridgejiang.github.io/Kekule.js/demos/chemFiles/3D/taxol.mol

TimErdmann87 commented 4 years ago

@amitsetty, the problem is caused by the js minification process in build. You have to manually set the some mangle configs for webpack. In the node_modules/react-scripts/config/webpack.config.js, please add a line of code in after line 234:

optimization: {
  ...
  minimizer: [
    ...
    new TerserPlugin({
      ...
      mangle: {
        safari10: true,   // line 234
        reserved: ['$super', '$origin']    // add this line of code
      }
      ...
    })
    ...
  ]
  ...
}

After that, the build should be OK.

Another approach is to use react-app-rewired to overwrite this configs rather than directly modifying it. Please refer to https://github.com/timarney/react-app-rewired.

@partridgejiang Thanks so much for your solution, you saved my a**!

filoscoder commented 1 year ago

@amitsetty, the problem is caused by the js minification process in build. You have to manually set the some mangle configs for webpack. In the node_modules/react-scripts/config/webpack.config.js, please add a line of code in after line 234:

optimization: {
  ...
  minimizer: [
    ...
    new TerserPlugin({
      ...
      mangle: {
        safari10: true,   // line 234
        reserved: ['$super', '$origin']    // add this line of code
      }
      ...
    })
    ...
  ]
  ...
}

After that, the build should be OK.

Another approach is to use react-app-rewired to overwrite this configs rather than directly modifying it. Please refer to https://github.com/timarney/react-app-rewired.

I been facing many import issues and as been treated in this thread I was suspecting some kind of trouble during minification proccess. I fix all issues with patch-package library. I recently tried mangle.reserved values to TerserPlugin with react-app-rewired, doing the same thing that patch-package do to react-scripts but nothing works. I'm not sure what is the reason, maybe kekule minification is related to react-scripts? (just a thought, idk)

Hope this help anyone with the same issue.