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

Add a hasCORS boolean property to the Exchange Structure /// Webpack throws error when building in Nuxt #225

Closed
kireerik opened this Issue Sep 21, 2017 · 36 comments

Comments

Projects
None yet
4 participants
@kireerik

kireerik commented Sep 21, 2017

I think it would be great to have a hasCORS boolean property in the Exchange Structure. So we would only need to define a proxy on the client side if it is really needed.

@kroitor

This comment has been minimized.

Member

kroitor commented Sep 21, 2017

Awesome idea! Thanks so much, adding it right away!

@kroitor kroitor self-assigned this Sep 21, 2017

kroitor added a commit that referenced this issue Sep 21, 2017

@kroitor

This comment has been minimized.

Member

kroitor commented Sep 21, 2017

Done! The flag will be available in new version 1.7.109+. Thx for your feedback!

@kroitor kroitor closed this Sep 21, 2017

@kroitor kroitor added the enhancement label Sep 21, 2017

@kireerik

This comment has been minimized.

kireerik commented Sep 22, 2017

Wow, thank you for adding it so fast!

I will update sometime, however with version 1.7.110 when I start my server in production mode, than I am getting the following error:

ERROR in pages\index.86b4ca59d1fc39bf44ed.js from UglifyJs
Unexpected token: name (CCXTError) [./node_modules/ccxt/ccxt.js:51,0][pages\index.86b4ca59d1fc39bf44ed.js:4503,6]
(node:6676) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 1): Error: Webpack build exited with errors

Maybe something got broken since version 1.7.1, because that works fine.

@kroitor

This comment has been minimized.

Member

kroitor commented Sep 22, 2017

@kireerik we have recently dropped support for very old versions of Node (since ccxt 1.7.x+), because they are outdated and don't have async/await and other modern JS features. So, if you have Node 5 or 6, please, update to 7.6.0+.

@kireerik

This comment has been minimized.

kireerik commented Sep 22, 2017

Thank you for your feedback! This issue occurs with Node.js 8.5.0.

@kroitor

This comment has been minimized.

Member

kroitor commented Sep 22, 2017

@kireerik ok, can you please try to reinstall the dependencies in node_modules from scratch?

rm -rf ./node_modules # VERY CAREFULLY
npm cache clean --force 
npm install

Does it still complain after that?

@kireerik

This comment has been minimized.

kireerik commented Sep 22, 2017

Sure, I have tried the same with Yarn (I am using it for performance reasons basically), but it doesn't make any difference.

I did some manual binary like search trying different versions and I can say that this issue was introduced with version 1.7.78 and it is working up to version 1.7.77.

@xpl

This comment has been minimized.

Member

xpl commented Sep 22, 2017

@kireerik the problem you are experiencing is related to UglifyJS that does not understand ES2015 and modern async/await syntax out of the box. You may consider adding Babel to your build pipeline (before UglifyJS), to transpile everything down to plain old ECMAScript5.

Here's how you can do it with Webpack: babel-loader.

You can also try switching to another minifier that supports ECMAScript6 and async/await, but if you're building a browser app and want to support older browsers, it would be better if you use Babel. Hope this helps!

@kireerik

This comment has been minimized.

kireerik commented Sep 22, 2017

Thank you @xpl for your comment!

I think I have ES6/ES7 Transpilation included with Nuxt.js. I am also using async functions and await operators in my project it self. So I guess this issue can be related to this library.

@xpl

This comment has been minimized.

Member

xpl commented Sep 22, 2017

@kireerik Can you please upload somewhere a minimal demo project (with Nuxt and everything set up) that demonstrates the error you experience?

@kireerik

This comment has been minimized.

kireerik commented Sep 22, 2017

We have a minimal example as well where the issue is reproducible too. You can request access from @cklester.

@kroitor

This comment has been minimized.

Member

kroitor commented Sep 22, 2017

@kireerik unfortunately, we can't access it on gitlab due to lack of permissions. If you're in contact with @cklester, can you please ask him to let us in? He's been very helpful to us previously. We could take a look to try and help fixing this bug / issue there. Thx!

@xpl

This comment has been minimized.

Member

xpl commented Sep 22, 2017

@cklester I've just registered as https://gitlab.com/xpl, can you give me access please?

@cklester

This comment has been minimized.

cklester commented Sep 22, 2017

@xpl Done! Thank you!

@xpl

This comment has been minimized.

Member

xpl commented Sep 22, 2017

@cklester Unfortunately, the adonuxt-ccxt-example project is still 404 for me:

screen shot 2017-09-22 at 23 16 48

@kroitor

This comment has been minimized.

Member

kroitor commented Sep 22, 2017

@kireerik , @cklester I see the same as @xpl does.

UPD. Looks like it was a cache problem with GitLab had to wait a little, and now @xpl is there.

@xpl

This comment has been minimized.

Member

xpl commented Sep 22, 2017

@cklester @kroitor Not quite... I have now access to ccxt-adonuxt-dashboard project, but the ccxt-adonuxt-example (with a minimal error repro case) mentioned by @kireerik is still not visible to me :(

@kireerik

This comment has been minimized.

kireerik commented Sep 22, 2017

Yes, as I can see @xpl got access to a little bit more complex project. I can technically give access for you to the more minimalistic one if @cklester allows me that (or he can do it to).

@xpl

This comment has been minimized.

Member

xpl commented Sep 22, 2017

@kireerik @cklester I would prefer investigating into less complex project, that would save time.

@cklester

This comment has been minimized.

cklester commented Sep 22, 2017

I've granted access for @xpl to the example repo.

@xpl

This comment has been minimized.

Member

xpl commented Sep 22, 2017

@kireerik I have troubles with reproducing the error... The steps I have performed so far on my MacOS:

Cloned the repo and installed dependencies with yarn:

git clone https://gitlab.com/kireerik/adonuxt-ccxt-example
cd adonuxt-ccxt-example
yarn install
yarn start

Then I had trouble with port 80 (not accessible on my system), so I've changed it to 3333 in the .env file, and then the launch succeeded with no errors. Here's the webpack log:

Hash: 0cd45b74de09c7762518
Version: webpack 3.5.6
Time: 15948ms
                                  Asset     Size  Chunks                    Chunk Names
                   img/logo.ffd9a8d.png  23.8 kB          [emitted]         
    pages/index.55e5c4f36a11cffa56df.js   453 kB       0  [emitted]  [big]  pages/index
layouts/default.ddcd2640e15ad7d4f7b8.js   1.6 kB       1  [emitted]         layouts/default
    pages/about.a0d446d59f9f483b197c.js  1.32 kB       2  [emitted]         pages/about
         common.b83a2094ee680ebe0eef.js   126 kB       3  [emitted]         common
            app.408cf97076731bc09cb1.js  33.5 kB       4  [emitted]         app
       manifest.0cd45b74de09c7762518.js  1.64 kB       5  [emitted]         manifest
                               LICENSES  2.04 kB          [emitted]         
 + 9 hidden assets

WARNING in asset size limit: The following asset(s) exceed the recommended size limit (300 kB).
This can impact web performance.
Assets: 
  pages/index.55e5c4f36a11cffa56df.js (453 kB)

Seems that the pages/index.55e5c4f36a11cffa56df.js file had the CCXT code included & minified, as expected:

screen shot 2017-09-23 at 01 33 00

@xpl xpl reopened this Sep 22, 2017

@xpl xpl closed this Sep 22, 2017

@xpl

This comment has been minimized.

Member

xpl commented Sep 22, 2017

@kireerik Hmm, looks like that the example project references a previous CCXT version. Will try changing it..

UPD: yep, now I see the error. Not sure what's causing it yet, though. Seems that it doesn't like the class keyword, so need to check whether the transpilation is configured correctly... I will investigate further.

@xpl

This comment has been minimized.

Member

xpl commented Sep 22, 2017

@kireerik I suspect that it is caused by a Webpack config where the node_modules folder is excluded from Babel, like in this issue. So will need to find where you can override this setting in AdonisJS (or Nuxt? Not sure...)

@xpl

This comment has been minimized.

Member

xpl commented Sep 23, 2017

@kireerik OK, seems I've found a way to override the Webpack config in Nuxt: How to extend webpack config?...

@xpl

This comment has been minimized.

Member

xpl commented Sep 23, 2017

@kireerik @cklester So here's the solution. Add the following to your config/nuxt.js file:

module.exports = {

  build: {
      extend (config, { dev, isClient }) {
        
        if (isClient) {

          config.module.rules.push ({
              test: /node_modules\/ccxt\/(.+)\.js$/,
              loader: "babel-loader",
              options: {
                babelrc: false,
                cacheDirectory: false,
                presets: ["es2015"],
                plugins: ["syntax-async-functions", "transform-regenerator"]
              }
          })
        }
      }
  },

This will add CCXT to the Babel transpilation stage. Seems that it could be a common problem across various frameworks. So if you're not the only ones who are having this problem, we will reconsider adding the Babel transpilation stage to the CCXT itself.

Hope this helps.

@kireerik

This comment has been minimized.

kireerik commented Sep 25, 2017

@xpl Thank you for your efforts!

I have tried the mentioned solution, but I still get the exact same error.

By the way did you made any braking changes in version 1.7.78?

@xpl

This comment has been minimized.

Member

xpl commented Sep 25, 2017

@kireerik What is your Webpack version? BTW, after adding the previously mentioned snippet to your Nuxt config, you should see this message in the log output:

[BABEL] Note: The code generator has deoptimised the styling of "/Users/mac/adonuxt-ccxt-example/node_modules/ccxt/ccxt.js" as it exceeds the max of "500KB".

It indicates that the transpilation is set up correctly and the CCXT source is going through Babel. If you don't see the message, something's wrong.

By the way did you made any braking changes in version 1.7.78?

We stopped using Babel, since the latest Node already supports all the syntax features we need, and there's no need in transpiling, and the browser people already use Babel on their ends... But it seems that even with that, some tools are just not yet ready for ES6 modules, as they expect everything in the node_modules to be pre-compiled to ES5.

If the problem persists, we will return Babel to our build pipeline, it's not a big problem.

@kireerik

This comment has been minimized.

kireerik commented Sep 25, 2017

As I can see it is 3.6.0. I am not getting such a notice. So this is working for you?

I see.

@xpl

This comment has been minimized.

Member

xpl commented Sep 25, 2017

@kireerik Yep it's working on my machine, with the adonuxt-ccxt-example project. Here's my adonuxt-ccxt-example/config/nuxt.js file:

'use strict'

const resolve = require('path').resolve

module.exports = {

  build: {
      extend (config, { dev, isClient }) {
        
        if (isClient) {

          config.module.rules.push ({
              test: /node_modules\/ccxt\/(.+)\.js$/,
              loader: "babel-loader",
              options: {
                babelrc: false,
                cacheDirectory: false,
                presets: ["es2015"],
                plugins: ["syntax-async-functions", "transform-regenerator"]
              }
          })
        } 
      }
  },

  /*
  ** Headers of the page
  */
  head: {
    title: 'Adonuxt',
    meta: [
      {
        charset: 'utf-8'
      },
      {
        name: 'viewport',
        content: 'width=device-width, initial-scale=1'
      },
      {
        hid: 'description',
        name: 'description',
        content: 'Adonuxt project'
      }
    ],
    link: [
      {
        rel: 'icon',
        type: 'image/x-icon',
        href: 'favicon.ico'
      }
    ]
  },
  /*
  ** Global CSS
  */
  css: ['~assets/css/main.css'],
  /*
  ** Customize the progress-bar color
  */
  loading: { color: '#744d82' },
  /*
  ** Point to resources
  */
  srcDir: resolve(__dirname, '..', 'resources')
}

What is the OS you use? If it's Windows, then the /node_modules\/ccxt\/(.+)\.js$/ regexp may fail because on Windows they use backward slashes. In this case, try changing it (the regexp).

@xpl

This comment has been minimized.

Member

xpl commented Sep 25, 2017

@kireerik Have you tried that fix with the example project? If you trying it with some other project, make sure you don't override the build property somewhere later in the config.

@kireerik

This comment has been minimized.

kireerik commented Sep 25, 2017

I see. Yes, this occurred on Windows. Now I have tried using resolve(__dirname, '..', 'node_modules/ccxt') as recommended and it somewhat works (/node_modules[\\\/]ccxt[\\\/](.+)\.js$/ works too).

However when I visit the application in the browser I am getting the following error in both development and production mode:
ccxt.js:2952 Uncaught (in promise) ReferenceError: regeneratorRuntime is not defined at Exchange.fetchTicker

So it seems ccxt is not working on the client side this way.

@xpl

This comment has been minimized.

Member

xpl commented Sep 25, 2017

@kireerik Hmm, seems that I forgot about the babel-polyfill. So here's the final (hopefully) fix:

module.exports = {

  build: {
      extend (config, { dev, isClient }) {
        
        if (isClient && !dev) {

          config.entry.common = ['babel-polyfill', ...config.entry.common]

          config.module.rules.push ({
              test: resolve(__dirname, '..', 'node_modules/ccxt'),
              loader: "babel-loader",
              options: {
                babelrc: false,
                cacheDirectory: false,
                presets: ["es2015"],
                plugins: ["syntax-async-functions", "transform-regenerator"]
              }
          })
        } 
      }
  },

It will add the required babel-polyfill dependency to your common scripts in the Webpack config. Let me know if it helped.

@xpl xpl changed the title from Add a hasCORS boolean property to the Exchange Structure to Add a hasCORS boolean property to the Exchange Structure / Issues with Uglify when using with Nuxt Sep 25, 2017

@xpl xpl changed the title from Add a hasCORS boolean property to the Exchange Structure / Issues with Uglify when using with Nuxt to Add a hasCORS boolean property to the Exchange Structure /// Webpack throws error when building in Nuxt Sep 25, 2017

@kireerik

This comment has been minimized.

kireerik commented Sep 25, 2017

Thank you! It works correctly this way.

@xpl

This comment has been minimized.

Member

xpl commented Sep 25, 2017

@kireerik UPD: Try also changing

if (isClient) {

to:

if (isClient && !dev) {

To disable the unnessesary transpilation in the development environment.

@xpl

This comment has been minimized.

Member

xpl commented Sep 25, 2017

@kireerik Sorry, had a mistake in my previous message (now edited). Transpilation makes build times considerably slower, so it is not a something you want in the dev mode...

@kireerik

This comment has been minimized.

kireerik commented Sep 25, 2017

Thank you!

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