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

Webpack Config is stripping out Crypto #1548

Closed
jmc265 opened this Issue Aug 4, 2016 · 64 comments

Comments

Projects
None yet
@jmc265
Copy link

jmc265 commented Aug 4, 2016

  1. OS?
    Mac OSX El Capitan
  2. Versions.
    angular-cli: local (v1.0.0-beta.10, branch: webpack)
    node: 4.4.7
    os: darwin x64
  3. Repro steps.
    I've pulled down the Angular-cli project and am building it locally, so I am using the new webpack version.
    The project that I am building uses Crypto libraries (via a dependency of a dependency ...).
    When I was running ng build I was seeing the dependency built like this:
    /* WEBPACK VAR INJECTION */(function(Buffer) {'use strict';

    var crypto = __webpack_require__(147);
    var BufferUtil = __webpack_require__(21);
    var $ = __webpack_require__(15);

    var Hash = module.exports;

    Hash.sha1 = function(buf) {
      $.checkArgument(BufferUtil.isBuffer(buf));
      return crypto.createHash('sha1').update(buf).digest();
    };
    ...

And then when I looked at module 147 (I.E. the crypto module) I was seeing nothing there:

/***/ },
/* 147 */
/***/ function(module, exports, __webpack_require__) {

/***/ },

After a while of searching I took a look in the Angular-CLI code and saw that part of the webpack config (addon/ng2/models/webpack-build-common.ts) was this:

node: {
      global: 'window',
      crypto: 'empty',
      module: false,
      clearImmediate: false,
      setImmediate: false
    }

When I remove crypto: 'empty' everything builds correctly with the crypto module getting pulled in.

So my question is, why have you specified that crypto should be empty?
Is there any way to fix this more permanently?

@jmc265

This comment has been minimized.

Copy link

jmc265 commented Aug 8, 2016

@TheLarkInn: Could you explain the crypto: 'empty' option added here:

@TheLarkInn

This comment has been minimized.

Copy link
Member

TheLarkInn commented Aug 8, 2016

Most projects by default aren't using crypto so removing that buildin results in smaller bundles. We'll implement a way to control this in the future.

@jmc265

This comment has been minimized.

Copy link

jmc265 commented Aug 8, 2016

I've had to fork the Angular CLI just to get my project to build properly. I don't think a build tool should be opinionated in this way. I spent over a day debugging to figure this out. Can you add the option in for this as soon as possible, I want to get off of my fork otherwise I am going to miss updates.

@filipesilva

This comment has been minimized.

Copy link
Member

filipesilva commented Aug 10, 2016

Unless I'm missing something here, it's kinda relevant to notice that we're building for the browser environment. Browser does not have crypto.

That we're using webpack to build the app, and that webpack is a node app is completely incidental: that could change to something else (SystemJS for instance) and your app would break as well.

I understand that you can't change your dependency of a dependency, but it doesn't sound like it would work in a non-node environment anyway. In that sense, I do not think we should include crypto.

@jmc265

This comment has been minimized.

Copy link

jmc265 commented Aug 10, 2016

That makes no sense, you are actively preventing anyone from building a Web app with crypto in it. Browsers do have crypto support: https://developer.mozilla.org/en-US/docs/Web/API/Window/crypto

@TheLarkInn, @filipesilva: If you won't re-open this issue to fix it, please at least suggest a workaround for people that want to use Cryptography in Angular.

(And BTW, this same project did build with the SystemJS version of Angular CLI)

@jmc265

This comment has been minimized.

Copy link

jmc265 commented Aug 15, 2016

@TheLarkInn, @filipesilva: Are you able to reply to the above?

@TheLarkInn

This comment has been minimized.

Copy link
Member

TheLarkInn commented Aug 15, 2016

@jmc265 so the node buildins are something that can vary per implementation of your app, and I see the merit of including them, however we need a clear and concise design around this proposal. If we expose these configuration properties, we need to do it in a way that will make sense to users, etc. What does this do? How do I use it? Why do I need it? What are the challenges etc.

Design Process: https://docs.google.com/document/d/1p_L-4RKY5PRMs9qdunpN56U66PTJSijfU2yb6V6KMuI/view

Linked above is our design process for creating a proposal for new features. I would certainly recommend you propose one for this feature for discussion.

@jmc265

This comment has been minimized.

Copy link

jmc265 commented Aug 15, 2016

@TheLarkInn I am really not understanding what you are asking for.
From my perspective, as a user of your build tool, all I am seeing is that I cannot build an app with cryptography in it, because of a limitation you specifically put in which I cannot configure nor override. You can see where I am coming from, can't you? It is so strange to me that a build tool has that limitation in, with the only reason being 'most projects don't use cryptography'. Well sure... But most projects don't use math.random, that doesn't mean you should forbid use of it in your build tool.

@jmc265

This comment has been minimized.

Copy link

jmc265 commented Aug 19, 2016

@TheLarkInn, @filipesilva: So you aren't going to fix this or provide any workaround? You are saying people who want to use crypto in their apps can't use Angular CLI...

@kylecordes

This comment has been minimized.

Copy link

kylecordes commented Aug 19, 2016

@jmc265 I think the most important part of the conversation above was "we'll implement a way to control this in the future.". Right now the CLI team is working real hard on supporting the most mainstream use cases. For example, CLI doesn't even have the ability to use the Angular template precompiler yet, or really any ability at all to add plug-ins or special configurations or anything else that doesn't match the default set up. I suggest perhaps bringing this issue back up some months from now, after that and numerous other mainstream cases are working well and in wide use. It'll probably be easier to get attention on corner cases at that time. You are correct above to say that, for the moment if you need a webpack or other setting beyond what CLI can do today, a good backup plan is to switch over to a (much more complex) "seed" project which drops the entire webpack configuration right in your project where you can edit it easily.

@jmc265

This comment has been minimized.

Copy link

jmc265 commented Aug 22, 2016

@kylecordes: Thank you, that make sense.
As this is an accepted issue which has not yet been fixed, could we move this ticket back into Open status so that it can be tracked?

@kylecordes

This comment has been minimized.

Copy link

kylecordes commented Aug 22, 2016

@jmc265 I'm just a drive-by, community, trying-to-be-helpful person using these technologies heavily, teaching some of them, reading lots of stuff, and sometimes commenting with explanations to help ease the workload of the team actually building this thing (so they can work more on building it). As for reopening issues... one of them would need to decide and comment on that.

@filipesilva

This comment has been minimized.

Copy link
Member

filipesilva commented Aug 23, 2016

I stand corrected: the browser environment does have crypto. Thus it makes sense to not bar it on a config level. I'll reopen this issue but don't have a timeline (nor strategy) for solving it.

Suggestions are welcome, but bear in mind we do not want to use node's crypto but instead use the browser one since it's part of the spec.

@jmc265 I'm also going to have to ask you to refrain from double posting though. I understand this issue is important for you but I get a load of notifications for this project, and more notifications do not make me get to issues faster. In fact, since I go over them in Google Inbox from oldest to newest, each time you double post the whole thread just gets moved to the top and thus it'll take longer to get seen.

@saulshanabrook

This comment has been minimized.

Copy link

saulshanabrook commented Oct 25, 2016

This issue is also affecting me. I am using the fernet library which uses the crypto.randomBytes in Node. By default, Webpack uses crypto-browserify to make this work on the browser.

@jmc265 You can still use the Web Cryptography API if you want to do crypto

@bpair

This comment has been minimized.

Copy link

bpair commented Mar 7, 2017

I am new to Angular and WebPack and for that matter Crypto but I struggled with this issue for two days before finding this reported issue. I think a good improvement would be to output to the console during the build to tell the user that that "Crypto" is being skipped. Also what is being skipped should be documented somewhere so it is easier to find. Since webpack supports crypto (and since AWS libraries now support webpack) I see more developers like me getting stuck with this. Thanks to @jmc265 for the workaround.

@carlosfaria94

This comment has been minimized.

Copy link

carlosfaria94 commented Mar 8, 2017

This issue is affecting me either. I'm tying to use crypto.createHash (bitpay/bitcore-lib#122). And it's supposed to be supported by webpack that uses crypto-browserify.

I agree that this decisions made by angular-cli team need to be documented. Is crypto being skipped? Please, say it.

@filipesilva

This comment has been minimized.

Copy link
Member

filipesilva commented Mar 13, 2017

@carlosfaria94 I'm closing your PR but adding the reasoning here so it's easier for people interested in this issue to follow.

I'm sorry but a change that exposes node's crypto will not go in at this moment. crypto-browserify is a partial implementation only. Web crypto might be coming but it's not here truly here yet.

Libraries that depend on crypto expect node's implementation. If it's meant to be used in the browser with Web Crypto API then there should be no problem with having node's crypto missing.

I understand that there are server-side libraries that kind of support working in the browser if crypto-browserify is available, but that's far from a standard. I don't think it's a good call in general to add it in the offchance some server-side libs won't completely break in the browser.

A possiblity in the near-ish future is to use a universal app instead. We're looking at adding some basic support soon.

@bpair

This comment has been minimized.

Copy link

bpair commented Mar 22, 2017

Is there a plan for a workaround? For example the ability to pass command line parameters to not skip Crypto?

We are using Heroku and with the current implementation it makes it extremely difficult to allow Heroku to be in charge of building the app.

I realize you do not consider a dependency on crypto a good practice or a standard implementation and that is fine but without a workaround it is difficult to use Angular-CLI in an automated build scenario.

@objectthink

This comment has been minimized.

Copy link

objectthink commented Mar 25, 2017

anything new on this? i am trying to use nats-io in an angular app create by the current angular-cli and require('nats') generates the error below. i suspect that this thread is relevant to this error.

nuid.js:73Uncaught TypeError: crypto.randomBytes is not a function

@intellix

This comment has been minimized.

Copy link
Contributor

intellix commented Mar 25, 2017

@bpair I'm currently using this package.json script to let Heroku build my app after it installs stuff:

"heroku-postbuild": "ng build --target=production -e ${NODE_ENV:-production} --aot"

It's not perfect as it's triggered when installing stuff, but it's not so bad and has worked for some time now :)

@theMattCode

This comment has been minimized.

Copy link

theMattCode commented Apr 23, 2017

Also affected here.

Tried to use gencryption which has a dependency to crypto. The lib calls createHash(...) which leads to Uncaught TypeError: crypto.createHash is not a function.

@bertofer

This comment has been minimized.

Copy link

bertofer commented May 14, 2017

Could this be an option on .angular.cli? I am affected too, and the solution already exists, and already exists on webpack using crypto-browserify. I see no reason to not allow that webpack functionality in angular-cli, even if it's set by default.

@lehtiniemi

This comment has been minimized.

Copy link

lehtiniemi commented Jun 15, 2018

@Asone Ah, yes. It is related to this: angular/devkit#967 but in this thread filipesilva redirected the discussion into this thread. Or maybe he just mentioned it's discussed here, in this case my mistake. But the issue seems to have the same change in Angular CLI as the root cause.

@George35mk

This comment has been minimized.

Copy link

George35mk commented Jul 7, 2018

I have the same error with tensorflow.js I can make any angular applications using the newest tesnsorflow versions.

the issue can be found here
tensorflow/tfjs#494

@adamayoung

This comment has been minimized.

Copy link

adamayoung commented Jul 30, 2018

Having similar problems when trying to build an Electron app using Angular 6 and AppAuth-JS. The AppAuth module is saying it can't resolve 'crypto' (along with a bunch of other modules).

Is there still no workaround for this?

@RedaKhyatti

This comment has been minimized.

Copy link

RedaKhyatti commented Jul 31, 2018

@admeister you could use the patch workaround. Create a patch.js file and make sure to run it postinstall

"postinstall": "node patch.js"

A patch file that works for me

const fs = require('fs');
const f = 'node_modules/@angular-devkit/build-angular/src/angular-cli-files/models/webpack-configs/browser.js';

fs.readFile(f, 'utf8', function (err,data) {
  if (err) {
    return console.log(err);
  }
  var result = data.replace(/node: false/g, 'node: {crypto: true, stream: true}');

  fs.writeFile(f, result, 'utf8', function (err) {
    if (err) return console.log(err);
  });
});
@carlosfaria94

This comment has been minimized.

Copy link

carlosfaria94 commented Aug 18, 2018

@filipesilva I do not believe this issue persist after 2 years.

You guys at angular-cli are killing the adoption of Angular in the development of distributed applications, which needs crypto to Web3.js work properly.

@tacnoman

This comment has been minimized.

Copy link

tacnoman commented Aug 23, 2018

I tried this and works

In webpack.config

module.exports = {
  // .....
  externals: {
    fs: 'commonjs fs',
    path: 'commonjs path',
    crypto: 'commonjs crypto'
  }
}

This solved (for me) a problem with fs and another problem with crypto.randomBytes

@clydin

This comment has been minimized.

Copy link
Member

clydin commented Aug 23, 2018

@carlosfaria94 By following the browser instructions for web3.js, it can be used successfully without any modifications. Specifically the CDN based approach which also has the advantage of simplified license compliance.

It is also important to note that crypto is not being stripped out of anything. The module only exists within the Node.js platform. Browser platforms do not have such a module. A cross-platform library should ideally contain versions targeting both of the major platforms. There is in fact a browser platform specific API for cryptography (details) which will (for major browsers) leverage the underlying operating system's cryptography support.

Also, if Node.js's crypto is absolutely required and a javascript version of the algorithms is acceptable to the project's requirements, crypto-browserify can be used in combination with a tsconfig path mapping to map crypto to the package.

@GrandSchtroumpf

This comment has been minimized.

Copy link

GrandSchtroumpf commented Aug 23, 2018

@carlosfaria94, if you want to use angular for Ethereum projects maybe you can use @ngeth instead of web3... It's built on top of angular and doesn't require crypto. It's still an early stage project, but you can already do JSON-RPC calls and use Contracts (bonus: it supports ABIEncoder V2).

@GrandSchtroumpf

This comment has been minimized.

Copy link

GrandSchtroumpf commented Oct 7, 2018

Hi guys, I found a more permanent way of making crypto available for angular 6.
We can use Custom Angular CLI builds with Angular 6 :

1 - install @angular-builders/custom-webpack :
npm i -D @angular-builders/custom-webpack

2 - Add custom builder in angular.json :
In angular.json > project > architect > build > builder replace current value with :
@angular-builders/custom-webpack:browser

3 - Create a file webpack.config.js at the root of the project :
This will be loaded by the new builder (by default the filename is webpack.config.js but you can have a custom one if needed see here.
Note : This will append your config to the default webpack config from angular.

4 - Add node support in webpack.config.js :
Here is what is needed for web3 for example.

module.exports = {
  node: {
    crypto: true,
    http: true,
    https: true,
    os: true,
    vm: true,
    stream: true
  }
}

Thanks a lot to Evgeny Barabanov for his amazing article on this subject.
Please read and clap his article if you think this is useful. There is a bonus inside that explains how to keep using ng serve with custom builders.

@clydin

This comment has been minimized.

Copy link
Member

clydin commented Oct 7, 2018

Please note that the project is not actually using node’s crypto (which uses the operating systems underlying implementations) with the settings. But rather a JavaScript partial implementation of the module. This applies to the others as well.

@studds

This comment has been minimized.

Copy link

studds commented Oct 22, 2018

@clydin thanks for the suggestion to use path mapping in tsconfig.json, works like a charm.

iantanwx added a commit to Zilliqa/Zilliqa-Wallet that referenced this issue Dec 12, 2018

chore(): fix npm-audit vulnerabilities
This fixes vulnerable build dependencies mainly by upgrading @angular/*
packages to updated versions. The angular project defintion has changed
accordingly, and incorporates a custom webpack config adapted from this
guide: angular/angular-cli#1548 (comment)

@kelsos kelsos referenced this issue Dec 14, 2018

Merged

Introduce web3 request batching #3

3 of 3 tasks complete
@JoeKahl

This comment has been minimized.

Copy link

JoeKahl commented Jan 2, 2019

We are suffering this same issue. After upgrading from Angular 2.0.0 to Angular 6.1.10 we are using the CLI for ng build and we have no more web pack. Our web app is using pdf and jpeg and tiff images heavily. We are using the tiff.js library and the ng build is resulting in:

ERROR in ./node_modules/tiff.js/tiff.min.js
Module not found: Error: Can't resolve 'crypto' in 'C:\Users\jkahl1\Source\Repos\servicecenter-fe\node_modules\tiff.js'
ERROR in ./node_modules/tiff.js/tiff.min.js
Module not found: Error: Can't resolve 'fs' in 'C:\Users\jkahl1\Source\Repos\servicecenter-fe\node_modules\tiff.js'
ERROR in ./node_modules/tiff.js/tiff.min.js
Module not found: Error: Can't resolve 'path' in 'C:\Users\jkahl1\Source\Repos\servicecenter-fe\node_modules\tiff.js'

We tried:
angular.json - "scripts": ["./node_modules/tiff.js/tiff.min.js"] and get the same results.
Is there a CLI only way to get this library to build?
Please provide guidance.

@GrandSchtroumpf

This comment has been minimized.

Copy link

GrandSchtroumpf commented Jan 2, 2019

Have you tried to use custom builders?

  1. Install the builders :
    npm install --save-dev @angular-builders/custom-webpack @angular-builders/dev-server.

  2. Update angular.json
    In angular.json change :

  • The builder of build (for ng build) with @angular-builders/custom-webpack:browser
  • The builder of serve (for ng serve) with @angular-builders/dev-server:generic
  1. Append the default webpack config
    Create a webpack.config.js file at the root of the project :
module.exports = {
  node: {
    path: true,
    crypto: true,
    fs: 'empty'
  }
}
@JoeKahl

This comment has been minimized.

Copy link

JoeKahl commented Jan 2, 2019

Thank you Francois. This got the build to complete without errors.
Many thanks. I hope our web architect approves of this. Seems okay to me.
Is fs: 'empty' because it is a server-side library?

@GrandSchtroumpf

This comment has been minimized.

Copy link

GrandSchtroumpf commented Jan 2, 2019

Sure, I'm glad it works. It's because there is no browser version of fs, which is good ^^. So you cannot use fs method in a browser. It's like child_process.

@bjohnson-va

This comment has been minimized.

Copy link

bjohnson-va commented Jan 8, 2019

@GrandSchtroumpf's solution worked for me as well. But I'll just add to the pile and say that it would be great to see first-party support for this customization.

@clydin

This comment has been minimized.

Copy link
Member

clydin commented Jan 8, 2019

@bjohnson-va there is first party support. please see the last paragraph of my comment above #1548 (comment) for a non-webpack specific solution.

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