Skip to content
This repository has been archived by the owner on May 1, 2020. It is now read-only.

Using CommonJS modules with rollup #16

Closed
shlomiassaf opened this issue Sep 21, 2016 · 83 comments
Closed

Using CommonJS modules with rollup #16

shlomiassaf opened this issue Sep 21, 2016 · 83 comments
Assignees

Comments

@shlomiassaf
Copy link
Contributor

shlomiassaf commented Sep 21, 2016

@adamdbradley I think you closed #3 in a rush.

angularfire2 and firebase are something you can't ignore when thinking ionic...

It's also not specific to them, it's a problem when using rollup, I think the rollup process needs to handle commonJS directories transparently.

I now have another, similar error with lodash

When I:

import { assign } from 'lodash';

I get:

[14:02:04]  bundle failed:  Error: Module /app/node_modules/lodash/lodash.js does not export assign (imported by /app/.tmp/data-source/some.service.js)

Again, there is a workaround but ionic developer shouldn't get hard time on simple tasks:

import assign from 'lodash/assign';

This is not perfect, there is a TS error:

[14:14:40]  src/data-source/feeding.service.ts(5,8): error TS1192: Module '"lodash/assign"' has no default export.

There's a switch in error between the 2, when TS is happy rollup isn't and vice versa.

BTW, rollup does a wired thing when doing:

import * as assign from 'lodash/assign';

assign will be the module, with a default property which is the assign function...

error_handler.js:45 EXCEPTION: Error in ./HomePage class HomePage - inline template:17:10 caused by: assign$2 is not a function

I know you considered #3 as someone else's problem but app-scripts is part of the ionic CLI and if the integration is not smooth you'll get ton's of complaints.
If you integrate rollup you need it to be transparent.

@shlomiassaf shlomiassaf changed the title PRoblem r Using ComminJS modules with rollup Sep 21, 2016
@danbucholtz
Copy link
Contributor

Hi @shlomiassaf,

We're not sure what you mean. We do support commonjs as we are using the latest version of the commonjs plugin.

https://github.com/driftyco/ionic-app-scripts/blob/master/config/rollup.config.js#L37

Please let me know if this fixes your issue. I'll re-open if needed.

Thanks,
Dan

@shlomiassaf
Copy link
Contributor Author

shlomiassaf commented Sep 21, 2016

@danbucholtz Why did you close the issue? is this a common behaviour for people contributing to your project? You didn't solve it...

Now, before you run and close try to look into the issue.

I kindly ask you to try and integrate angularfire2 or lodash.

I'm trying to save you a lot of time and complaints later on, I'v spent a lot of hours on this beta, this is not how you should treat people who try to help.

This is insulting

Jus to clarify, the reason I react like that is that the same error was closed in #3 without a solution...

@shlomiassaf
Copy link
Contributor Author

shlomiassaf commented Sep 21, 2016

Here's another issue for you:

In watch mode when using skip property in the nodeModules plugin for rollup (within rollup.config.js) the skip is only valid for the first run.

Save something and the next update will error since using cache for rollup causes an issue.
If you disable caching everything is fine, but you can't do that without changing the code of this lib.

I'v managed to workaround the angularfire issue and now this cache thing you introduced broke it...

I know sometime people pollute the issues but this is not the case, anyone who could use the beta.12 without docs should be taken seriously, its across 4 repos of integration.

@danbucholtz danbucholtz reopened this Sep 21, 2016
@danbucholtz
Copy link
Contributor

Hi,

I am seeing an issue here. I'll look into it. The goal is the user doesn't need to care if a 3rd party lib is commonjs or es2015 modules.

Thanks,
Dan

@shlomiassaf
Copy link
Contributor Author

I know it's the goal and I also now that you'll get tons of complaints for these 2 libs only...

I'm trying to help.

@mlynch
Copy link
Contributor

mlynch commented Sep 22, 2016

@shlomiassaf we're looking into it and taking it seriously. This needs to Just Work or it won't be acceptable on our end. Apologies for the confusion.

@shlomiassaf
Copy link
Contributor Author

shlomiassaf commented Sep 26, 2016

@danbucholtz @mlynch This is really a core issue with the rollup bundling process.
I can't import major libraries.

Now moment is giving me hard time, see this issue comment, I can run in dev but I can't build to a device.

BTW, I had to use moment since the Intl polyfil is not present so iOS safari can't run angular 2 if for example i'll use the date pipe.

@danbucholtz
Copy link
Contributor

Hi @shlomiassaf,

We are aware of this issue and working on it. I don't have more to share right now but I hope to soon.

Thanks,
Dan

@danbucholtz
Copy link
Contributor

Hi @shlomiassaf,

I have successfully created a project with moment, lodash, node-uuid, localforage. All commonjs modules. You're right there is an issue here. I've come up with a solution. I'll have more details on it tomorrow.

In the meantime, the code changes to app-scripts to facilitate it are here:
#29

Thanks,
Dan

@shlomiassaf
Copy link
Contributor Author

@danbucholtz thanks!

I couldn't load moment via

import * as moment from 'moment'

I had to use default syntax:

import moment from 'moment'

This requires

    "allowSyntheticDefaultImports": true,

in tsconfig.json

Problem is i'm getting a new error now, in runtime...
I'm pretty sure it's not related to your change but if you have clues it will be great

EXCEPTION: Error in ./IonicApp class IonicApp - inline template:0:34 caused by: No provider for ParamDecorator!ErrorHandler.handleError @ error_handler.js:45
error_handler.js:47ORIGINAL EXCEPTION: No provider for ParamDecorator!ErrorHandler.handleError @ error_handler.js:47

@danbucholtz
Copy link
Contributor

@shlomiassaf,

Nice find with the allowSyntheticDefaultImports option. I'll play around with it - this sounds like the secret sauce that we need to make things work the way we want!

I'll have an update coming soon.

Thanks,
Dan

@danbucholtz
Copy link
Contributor

@shlomiassaf!

That option is working great so far!

Thanks,
Dan

@danbucholtz
Copy link
Contributor

I made my sample project public. Check it out.

https://github.com/driftyco/ionic-third-party-lib-example

Thanks,
Dan

@shlomiassaf
Copy link
Contributor Author

@danbucholtz Thanks! great progress.

It looks like the bundling issue is solved.

The cache issue is still there.

The node UUID lib uses the 3rd partycrypto library that rollup treats as external dependency.
On the first run everything is fine but any code update done in watch mode will result in an error:

[21:23:35]  Error: Could not load crypto (imported by /Users/shlomiassaf/Desktop/Cisco/IONIC/ionic-third-party-lib-example/node_modules/node-uuid/uuid.js): ENOENT: no such file or directory, open 'crypto'

If we disable caching of the rollup bundle by commenting out this line code update works without a problem.

So, the problem is that caching somehow does not respect previous configuration.

For me this happened when using firebase as external module via the nodeModules plugin in rollup:

    nodeResolve({
      module: true,
      jsnext: true,
      main: true,
      browser: true,
      extensions: ['.js'],
      skip: [ 'firebase' ]
    })

@shlomiassaf
Copy link
Contributor Author

@danbucholtz, still can't use angularfire2

@shlomiassaf
Copy link
Contributor Author

@danbucholtz keep an eye on angular/angularfire#565

@awe-sim
Copy link

awe-sim commented Sep 29, 2016

Hi @danbucholtz,

I am facing similar issues over using external libraries like lodash.. I tried cloning the project you linked earlier (https://github.com/driftyco/ionic-third-party-lib-example).. The application runs and the libraries seem to work fine.. But in my IDE (SublimeText), I'm getting the following issues:

  1. For each Component ts file: Experimental support for decorators is a feature that is subject to change in a future release. Set the 'experimentalDecorators' option to remove this warning.
  2. For each of the library imports: Module "....../ionic-third-party-lib-example/node_modules/moment/moment" has no default export. In addition, IDE autocomplete for these library symbols is not working..

It seems to me I'm doing something stupid and overlooking something simple.. Could you please help in this regard?

@danbucholtz
Copy link
Contributor

I'm thinking the sublime plugin you're using likely does not support the latest major version of Typescript (2.0). Give vscode a shot, it is maintained by Microsoft so it works incredibly well with Typescript.

Thanks,
Dan

@shlomiassaf
Copy link
Contributor Author

shlomiassaf commented Sep 29, 2016

@awe-sim As a rule of thumb I always check console errors when running the complication and only then take the IDE errors seriously. No error in console, no issue.

It's hard for IDE plugins to keep up when changes are so often :)

@awe-sim
Copy link

awe-sim commented Sep 30, 2016

@danbucholtz .. Thanks for pointing the issue out.. Yes the sublime plugin supported typescript v <2.0.. Using VSCODE now and its working good.. Thanks a lot.. =)

@danbucholtz
Copy link
Contributor

@shlomiassaf, any leads on what's going on with AngularFire? Sounds like they may use a dependency that rollup can't handle? We may need to make WebPack an option if Rollup can't handle all libs. That would be unfortunate as performance would suffer.

Thanks,
Dan

@joshgarwood
Copy link

I'm also having a similar issue with moment-timezone. I would love it if you guys reverted to webpack until rollup is more seriously vetted. The collective amount of time the community has wasted battling with this new build system is a rough indictment of the quality of this release. That, coupled with the general confusion on importing styles, iOS build failures with ionic package, et al... is just not consistent with the quality you guys typically ship with :/

Sorry to rant here, as I know it's not the place; it's just be a super frustrating 48 hours trying to migrate my beta11 project to rc0.

@shlomiassaf
Copy link
Contributor Author

@danbucholtz I think you should have a talk with the firebase guys.

I'm not sure the issue is with the rollup build process, and if it is it's unique to the firebase build...

Just to recap I will sum up...
There are 2 issues, one related to firebase the other is not.

Rollup can't handle firebase.

It should be noted that this happens when firebase is imported from angularfire2 I have not checked what happens if we omit angularfire2 and try to import firebase alone.
@danbucholtz This should hint, if you can go with firebase without angularfire2 you should go to the angularfire2 guys and vice versa :)

To workaround for now see this issue comment

Rollup watch not working when using skip option in rollup-plugin-node-resolve

Basically I used skip to workaround the firebase issue.
The problem is the cache bundle implemented in the bundle process, when cache is disabled there is no issue. When cache is enabled 1st compilation works fine but when updating the code rollup throws that he can't find firebase

Hope it helps, Thanks.

@shlomiassaf
Copy link
Contributor Author

shlomiassaf commented Oct 2, 2016

@mlynch & @danbucholtz About your note, moving to Webpack instead of Rollup.
I think it is the right place to go to and should have been used from the start.

  • Webpack is the best tool today for development, it will also be the fastest on live reload.
  • Webpack has AoT compilation plugin (built by @robwormald) that is not coupled to the cli.
  • Webpack has tree shaking (performance....)
  • Webpack is the de-facto tool for angular developers, the CLI uses it.
  • Webpack supports lazy loading with the angular router, out of the box.
  • Webpack's new DLL plugin will reduce build times since all ionic-angular will live there.

There are lot's of other reasons, really so many.

the app-scripts package is trying to implement things webpack does and does great!
I don't see any benefit of using a custom rollup procces when it's clear that the direction of the community (at least angular) is webpack.

Why spending the time exploring lazy loading with your custom process or caching or any other issue when Webpack has it already baked in and if not the angular cli team has probably tackled it and solved it...

The angular team is going to release a lot of tools used by the Angular CLI as decoupled webpack plugins (see npm namespace @ngtools), this is what they said in angular connect.

I really thing you should reconsider and move to webpack.

BTW: My angular 2 library is using webpack for development and Rollup for UMD bundling so I can deploy an AOT ready library and an ES5 (UMD) library... It's possible to integrate the 2

@OnnoGabriel
Copy link

@danbucholtz Thanks for writing a doc on external library usage. With the Ionic 2 beta releases I had also problems with Chart.js ([http://www.chartjs.org/]) and ended up with a quick-and-dirty load in index.html. I could not test it with RC0 until now, because I am still struggling to get PouchDB started, which was fine in the Betas... For now, I will stop with this Ionic 2 project and wait until third party libs are more easily loaded into Ionic 2.

@sosnet
Copy link

sosnet commented Oct 5, 2016

I have problem with angular-in-memory-web-api: angular/in-memory-web-api#42

@danbucholtz
Copy link
Contributor

danbucholtz commented Oct 5, 2016

All,

We've posted some docs on the subject. Note: They match a version of @ionic/app-scripts that will be dropped in the next day or so. 95% of the concepts still apply.

http://ionicframework.com/docs/v2/resources/third-party-libs/
http://ionicframework.com/docs/v2/resources/app-scripts/

Here's an example with AngularFire2.
Here is a project I implemented that has most libraries in it that Ionic users have reported trouble with. Most of the libraries worked, but a couple did not. Here is the summary of the findings.

Please let me know how everything goes. I'm going to close this issue for now as we've shipped the documentation.

Thanks,
Dan

aggarwalankush referenced this issue in ionic-team/ionic-conference-app Oct 6, 2016
@luchillo17
Copy link

@danbucholtz i'm seeing you have that example with lodash, however what about lodash modules?

Say i'm importing it like this:

import { map, reduce } from 'lodash';

Then the freaking cli complains:

bundle dev failed:  Module /home/carlos/Carlos/OSGroup/startApp/node_modules/lodash/lodash.js does not export map (imported by /home/carlos/Carlos/OSGroup/startApp/src/providers/db.ts)

[12:27:34]  Error: Module /home/carlos/Carlos/OSGroup/startApp/node_modules/lodash/lodash.js does not export map (imported by /home/carlos/Carlos/OSGroup/startApp/src/providers/db.ts)

This was working with my Webpack setup right before i started transitioning to Rollup.

@Kobzol
Copy link

Kobzol commented Oct 6, 2016

Try this:
import _ from "lodash";
_.map(...)
Some of the imports have to be modified to work with Rollup.

@danbucholtz
Copy link
Contributor

@Luchillo,

@Kobzol is correct. If it's a third party module, it's best to just import the whole thing for now since most are commonjs. See our docs on third party libraries here.

Thanks,
Dan

@danbucholtz
Copy link
Contributor

@shlomiassaf, you figure out the cache thing? I am trying to come up with a solution for it right now.

Thanks,
Dan

@luchillo17
Copy link

@Kobzol Yes i know that can be done, the objective of importing the individual modules was to limit the size of the lodash import, maybe three shaking takes care of it, idk but still this should work.

@danbucholtz for the time being i'm going to import the full lodash lib, is there any issue to track about those kind of sub-modules with Rollup or this repo?

@danbucholtz
Copy link
Contributor

danbucholtz commented Oct 6, 2016

@Luchillo,

You're importing the entire lodash lib anyway because it it is a commonjs module. Commonjs modules aren't tree shakable.

If you want everything to just work nicely, lodash-es is the es2015 version. See link here.

In general, if a named export like this import { capitalize } from 'lodash'isn't "just working" out of the box, you're importing the entire library whether you want to or not.

The JS community is right on the verge of a massive shift from commonjs to go all in on the standard ES2015 modules. Once Node adopts it, which they are committed to and soon, it will be game on and ES2015 will be off the races.

We just need to get through the next few months to get there.

Thanks,
Dan

@luchillo17
Copy link

luchillo17 commented Oct 6, 2016

Ok so lodash-es is the next version, does it works with sub-modules (or however it is called)?

Also i don't see the @types for lodash-es, will the regular ones work?

Ok no type definitions, but at least i just had to find->replace the imports in 27 files on my project, i can't begin to imagine how many places would i have to go and change manually without lodash-es.

@luchillo17
Copy link

luchillo17 commented Oct 6, 2016

Now here start my hell, i had a whole range of libs in my Webpack config, how am i supposed to import this libs or their equivalent in Rollup?:

  • Define a variable's value in global scope.
  • intl
  • Chart.js.
  • ng2-charts
  • web-animations-js
  • signature_pad.
  • comparev.
  • font-awesome (Previously done using the font-awesome-webpack package).

I also have 1 custom font i need to copy/paste, is that in the assets folder?

@danbucholtz
Copy link
Contributor

@Luchillo,

I don't think your code will have to change much at all. For the most part, anything that was like this:

import * as something from 'someLib'

should become:

import something from 'someLib'

If the fonts go into a folder in assets it will automatically be copied over.

Thanks,
Dan

@luchillo17
Copy link

luchillo17 commented Oct 6, 2016

Ok that leaves moment, lodash, ng2-charts all green.

However intl, web-animations-js were called in a polyfills.ts file and in webpack was a separate entry, how would i do the same in Rollup, do i have to make a Rollup config file?

My webpack config used the ProvidePlugin to provide chart.js, signature_pad and comparev as global scope, just like an html script tag would load those libs, how to in Rollup?

Lastly i need to define a variable, what i do is to require the package.json, then use the version attribute to set the VERSION variable in the app's global scope with webpack's ProvidePlugin, Rollup?

May i need to copy rollup.config.js and start hacking?

@danbucholtz
Copy link
Contributor

@Luchillo,

Here is the documentation on customizing the build process. So far we only have it populated for bundling, which is what you'll want.

http://ionicframework.com/docs/v2/resources/app-scripts/

Thanks,
Dan

@luchillo17
Copy link

@danbucholtz So i started editing my Rollup config, it compiles with only a few warnings about unused vars, however in browser this error pop's in console:

Uncaught ReferenceError: IntlPolyfill is not defined

Which i've tested and it has an error with my polyfills that i'm trying to import in app.module,ts as:

import './polyfills';

I've tested commenting the imports in that file and error changes depending the lib that is first in the file, if i comment intl, then web-animations-js polyfill breaks, so this is not the way to import polyfills?

@danbucholtz
Copy link
Contributor

@Luchillo,

Maybe it is best to create a post on the Ionic forum. We're happy to help but I'm not sure this is the correct medium with all of the other subscribers.

I would need to see your code to help further. I don't have the necessary context.

Thanks,
Dan

@luchillo17
Copy link

Yes, i already have, however the response time of anyone that can really help in this new issues is close to 2+ weeks, here's the post, it has more context about how am i trying to import, i don't think is that complex to reason about: https://forum.ionicframework.com/t/how-to-rollup-pollyfils/66108

@alexmgrant
Copy link

alexmgrant commented Oct 7, 2016

Hey folks, I have a couple projects with this setup so I created a post about how to get it working. Not ideal, but until the Firebase team updates their code base it's the best I could do.

http://alexmgrant.github.io/ionic/firebase/rc0/lodash/2016/10/06/ionic-rc0-&-firebase.html

@danielcrk-cn
Copy link

@danbucholtz I'm having issues with ImmutableJS, getting a .../immutable.js does not export fromJS error on build. Any advice?

@danbucholtz
Copy link
Contributor

danbucholtz commented Oct 10, 2016

I was able to import and use immutable. If you want to use namedExports like fromJS, you'll need to provide a custom Rollup config as Immutable is not distributed as a modern/standard ES2015 library despite being written as one.

See here for an example, and see our docs on third party libs here.

See the bundle section here.

Thanks,
Dan

@parliament718
Copy link

I'm having trouble using d3 and d3-request specifically.

According to Mike Bostock the bundle process "is not importing the code from the ES2015 source, per the module (and jsnext:main) entry point defined in the package.json. Instead it appears to be pulling from the generated UMD bundle"

If anyone could help me resolve this, there's a 100 bounty on my stackoverflow question which I'm almost certain is related to this issue.

http://stackoverflow.com/questions/39909200/d3-4-0-import-statement-gives-moduleexports-wrapper

@danbucholtz
Copy link
Contributor

danbucholtz commented Oct 10, 2016

@parliament718,

d3 has a strange default export, so I had to use the full module import syntax. It is important to note that in this case, d3 is a "wrapper" object that follows the es2015 standard. It is an object containing reference to all of the exports. d3.default would be the default export for example.

import * as d3 from 'd3';
console.log('d3: ', d3);

The log statement should reveal what fields it has and the docs should reveal how to use them.

We are definitely importing the code from module, then jsnext:main, then main in that order.

Thanks,
Dan

@danbucholtz
Copy link
Contributor

Hello all,

Just as a follow up, we have a PR open right now adding Webpack support. Webpack 2 beta has come a long way and the start-up time gap is not nearly as long as it once was.

Hopefully this will be released sometime next week.

Thanks,
Dan

@luchillo17
Copy link

@danbucholtz Man i'm glad to hear that, i'll be the first (or at least i'll try) to implement it when the PR get's merged.

@gulukul
Copy link

gulukul commented Oct 16, 2016

@danbucholtz I checked all the links and also your mega test. I am trying to use https://github.com/videogular/videogular2

The instructions to install and use it are here:
https://github.com/videogular/videogular2/blob/master/docs/getting-started.md

It already runs in Angular 2 using TypeScript.
This is used for videos on iOS. I just want to show video on autoplay on iOS and am not able to find a native plugin, so thought I would integrate videogular2. But I a not able to do so. Can you please take a look at this too?

Thanks

@zhakhalov
Copy link

zhakhalov commented Oct 16, 2016

I have same issues.

Ionic bundles succesfully in case of 'out of the box' but customizing is painful.
I found not documented --dev flag which only ables development build for ionic run.

Rollup still has lot of issues with external libraries. I cannot understand why it was choosen.

I tried to use webpack with Ionic2RC1 in my project.

  • ts-loader just stuck at 49%
  • awesome-typescript-loader built Ionic with a lot of errors. Result build was unrunnable.

I even tried to build Ionic2 from sources but I've met runtime error
Uncaught Error: Can't resolve all parameters for provideLocationStrategy: (PlatformLocation, ?, Config).

Few words about performance.

Webpack has code-splitting and CommonChuncks plugin which significaly improve development performance.
e.g
~700ms watch-rebuild for Webpack
~7s watch-rebuild for Rollup

Few words about gulp

It is not a bundling tool now. It is tusk runner, for every job you lazy to do manually, from bundling application with webpack or any other bundler to signing android release APK and opening Xcode.

@deepakshakya
Copy link

deepakshakya commented Oct 18, 2016

@dan: Thanks for adding webpack support. I am eagerly waiting for this support :-)
Current I am seeing issue as mentioned below:-

`
18 10 2016 22:04:35.664:INFO [karma]: Karma v0.13.22 server started at http://localhost:9876/
18 10 2016 22:04:35.674:INFO [launcher]: Starting browser Chrome
18 10 2016 22:04:37.638:INFO [Chrome 53.0.2785 (Mac OS X 10.12.0)]: Connected on socket cZvudWwQ4tXebfJvAAAA with id 5405126
Chrome 53.0.2785 (Mac OS X 10.12.0) ERROR
Uncaught Error: Cannot find module "./data/packed/latest.json"
at /Users/gyana/Sites/gyana-ai/tests.webpack.js:49281 <- webpack:////moment-timezone/index.js:2:0
Chrome 53.0.2785 (Mac OS X 10.12.0) ERROR
Uncaught Error: Cannot find module "./data/packed/latest.json"
at /Users/gyana/Sites/gyana-ai/tests.webpack.js:49281 <- webpack:///
/moment-timezone/index.js:2:0
Chrome 53.0.2785 (Mac OS X 10.12.0): Executed 0 of 0 ERROR (0.7 secs / 0 secs)

Finished in 0.7 secs / 0 secs
`

I assume this will be addressed with your webpack support to moment-timezone?

Thanks,
Deepak.

@danbucholtz
Copy link
Contributor

danbucholtz commented Oct 19, 2016

We have entered a beta testing phase with Webpack.

Please see this post if interested in testing.
https://forum.ionicframework.com/t/help-us-test-webpack-support-in-ionic-2-app-scripts/67507

Specifically, we're looking for library incompatibility and additional loaders we may need to add. We are working on improving the performance of the builds, too.

We are only using Webpack as a javascript bundler - not as the development server, other asset bundler, etc. We intend to support Rollup going forward as well.

Thanks,
Dan

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

No branches or pull requests