feature(webpack): webpack bundling support #158

danbucholtz · 2016-10-14T07:04:36Z

No description provided.

adamdbradley · 2016-10-14T14:20:41Z

config/bundle.config.js

@@ -0,0 +1,3 @@
+module.exports = {


Let's not add another config here. Instead let's have webpack the default, and if a custom rollup config is provided, then to use rollup instead.

adamdbradley · 2016-10-14T14:22:12Z

src/bundle.ts

-  return rollup(context, configFile)
-    .catch(err => {
+  return createBundle(context, configFile, bundleConfig)
+    .catch((err: Error) => {


It's prettier without Error and that's how it's used everywhere else.

adamdbradley · 2016-10-14T14:22:35Z

src/bundle.ts

+function createBundle(context: BuildContext, configFile: string, bundleConfig: BundleConfig) {
+  if (bundleConfig.useWebpack) {
+    return webpack(context, configFile);
+  } else {


No else here

Yep, the if->return should be enough to take care for the else case.

adamdbradley · 2016-10-14T14:22:41Z

src/bundle.ts

+      .catch( (err: Error) => {
+        throw new BuildError(err);
+      });
+  } else {


adamdbradley · 2016-10-14T14:23:05Z

src/bundle.ts

+  const bundleConfig = getBundleConfig(context, null);
+  if (bundleConfig.useWebpack) {
+    return webpackUpdate(event, path, context, null)
+      .catch( (err: Error) => {


Just .catch(err => { no extra spaces like the rest of Ionic

adamdbradley · 2016-10-14T14:23:26Z

src/bundle.ts

+      });
+  } else {
+    return rollupUpdate(event, path, context)
+      .catch( (err: Error) => {


.catch(err =>

adamdbradley · 2016-10-14T14:23:53Z

src/bundle.ts

-  const rollupConfig = getRollupConfig(context, null);
-  return rollupConfig.sourceMap;
+  const bundleConfig = getBundleConfig(context, null);
+  if (bundleConfig.useWebpack) {


Change this so if a custom rollup config was provided, then use rollup, otherwise always use webpack.

adamdbradley · 2016-10-14T14:24:04Z

src/bundle.ts

+  if (bundleConfig.useWebpack) {
+    const webpackConfig = getWebpackConfig(context, null);
+    return webpackGetOutputDest(context, webpackConfig);
+  } else {


adamdbradley · 2016-10-14T14:24:10Z

src/bundle.ts

+  if (bundleConfig.useWebpack) {
+    // TODO - read this from webpack config (could be multiple values)
+    return true;
+  } else {


adamdbradley · 2016-10-14T14:24:50Z

src/bundle.ts

+  return bundleConfig;
+}
+
+const taskInfo: TaskInfo = {


Let's not create a new bundle config

adamdbradley · 2016-10-14T14:25:28Z

src/loaders/ionic-loader.ts

+
+const transpiledFiles = getCachedTranspiledTsFiles();
+
+module.exports = function ionicLoader(source: any, map: any) {


Let's name this ionCompilerLoader

And rename the file to ion-compiler-loader

adamdbradley · 2016-10-14T14:26:31Z

src/webpack.ts

+import { BuildError, Logger } from './util/logger';
+import { fillConfigDefaults, generateContext, getUserConfigFile, replacePathVars } from './util/config';
+import { isAbsolute, join } from 'path';
+


no new line

adamdbradley · 2016-10-14T14:28:54Z

src/webpack.ts

+}
+
+
+export function createWebpackBundle(context: BuildContext, configFile: string): Promise<any> {


rename to webpackWorker

TheLarkInn · 2016-10-14T21:47:24Z

Looking good :)

rkostrzewski · 2016-10-15T14:17:23Z

Hi @danbucholtz, I've just used ionic-app-scripts from this branch to check out if it will work. I'm getting error in sass transform part. Using rollup works just fine.

The error I'm getting is:

[15:55:00]  sass failed: Path must be a string. Received null                                                              
[15:55:00]  ionic-app-script task: "build"                                                                                 
[15:55:00]  TypeError: Path must be a string. Received null

I've traced the error to generateSassData function in sass.ts anonymous function at line 111. Webpack bundling process seems to add null to context.moduleFiles. Simple filtering solves the problem: context.moduleFiles.filter(m => !!m).forEach(...). Apart from that everything works out of the box for all my projects 👍

danbucholtz · 2016-10-17T21:27:55Z

src/webpack.ts

+        } else {
+          // set the module files used in this bundle
+          // this reference can be used elsewhere in the build (sass)
+          const files = stats.compilation.modules.map((webpackObj: any) => {


@TheLarkInn, any idea why resource might be null on some modules when I loop over them? I am filtering it out or just using context for my purposes, but it was surprising to have resources populated correctly 999/1000ish times.

Thanks,
Dan

marsanla · 2016-10-19T09:07:07Z

Publish in NPM last version
Thanks :)

danbucholtz · 2016-10-19T16:33:01Z

@marsanla,

Soon. It's not quite ready yet.

Thanks,
Dan

mehrshadpezeshk · 2016-10-21T08:27:15Z

Hello,

I'm trying this pull request and i get this error :

Running 'run:before' npm script before run

alopeyk-customer-app@ watch /Users/mehrshadpezeshk/Sites/alopeyk/customerapp
ionic-app-scripts watch

module.js:457
throw err;
^

Error: Cannot find module '../dist/index'
at Function.Module._resolveFilename (module.js:455:15)
at Function.Module._load (module.js:403:25)
at Module.require (module.js:483:17)
at require (internal/module.js:20:19)
at Object. (/Users/mehrshadpezeshk/Sites/alopeyk/customerapp/node_modules/@ionic/app-scripts/bin/ionic-app-scripts.js:19:3)
at Module._compile (module.js:556:32)
at Object.Module._extensions..js (module.js:565:10)
at Module.load (module.js:473:32)
at tryModuleLoad (module.js:432:12)
at Function.Module._load (module.js:424:3)

npm ERR! Darwin 16.0.0
npm ERR! argv "/usr/local/bin/node" "/usr/local/bin/npm" "run" "watch" "--color"
npm ERR! node v6.7.0
npm ERR! npm v3.10.3
npm ERR! code ELIFECYCLE
npm ERR! alopeyk-customer-app@ watch: ionic-app-scripts watch
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the alopeyk-customer-app@ watch script 'ionic-app-scripts watch'.
npm ERR! Make sure you have the latest version of node.js and npm installed.
npm ERR! If you do, this is most likely a problem with the alopeyk-customer-app package,
npm ERR! not with npm itself.
npm ERR! Tell the author that this fails on your system:
npm ERR! ionic-app-scripts watch
npm ERR! You can get information on how to open an issue for this project with:
npm ERR! npm bugs alopeyk-customer-app
npm ERR! Or if that isn't available, you can get their info via:
npm ERR! npm owner ls alopeyk-customer-app
npm ERR! There is likely additional logging output above.

npm ERR! Please include the following file with any support request:
npm ERR! /Users/mehrshadpezeshk/Sites/alopeyk/customerapp/npm-debug.log
Caught exception:
undefined

Mind letting us know? https://github.com/driftyco/ionic-cli/issues

Thanks

danbucholtz · 2016-10-21T14:01:07Z

@mehrshadpezeshk,

Run npm install @ionic/app-scripts@beta. That'll pull down the webpack version.

Thanks,
Dan

mehrshadpezeshk · 2016-10-21T14:04:09Z

Thanks @danbucholtz it worked

tonyawad88 · 2016-10-22T12:53:07Z

Hi Dan, It takes about 12seconds for the webpack to bundle. Is this normal ?@danbucholtz

luchillo17 · 2016-10-22T13:20:19Z

For the first run? it's very fast.

For subsequent runs? it's damn slow.

I have a bundle process with webpack and Ionic 2 beta.11 and first run most of the time takes 20~30 depending if i'm in a machine with SSD or not, while subsequent runs take max 4 seconds in non SSD machines.

tonyawad88 · 2016-10-22T13:23:42Z

@Luchillo All of mine are taking 12seconds + (running on macbook pro with ssd)

[09:20:03]  transpile update started ... 
[09:20:03]  transpile update finished in 277 ms 
[09:20:03]  webpack update started ... 
JS changed:   www/build/main.js
[09:20:17]  webpack update finished in 13.20 s  <=========
[09:20:17]  watch ready 
[09:20:17]  transpile update started ... 
[09:20:18]  transpile update finished in 967 ms 
[09:20:18]  webpack update started ... 
JS changed:   www/build/main.js
[09:20:33]  webpack update finished in 15.88 s <=========
[09:20:33]  watch ready

luchillo17 · 2016-10-22T13:38:30Z

Which version of webpack, are you using awesome-typescript-loader?

tonyawad88 · 2016-10-22T13:42:15Z

It's version: "2.1.0-beta.25"

I basically ran this: npm install @ionic/app-scripts@beta as per @danbucholtz

luchillo17 · 2016-10-22T13:48:41Z

Hmm, i'm about to test this new ionic-app-scripts beta, however can't find any docs on it, i'll be trying things for now.

luchillo17 · 2016-10-22T14:46:30Z

Anyone knows what's up with that ionic-loader? i don't seem to understand how it works since all i see in it's source code is some sort of caching mechanism.

jaspal27 · 2016-11-03T03:55:26Z

"@ionic/app-scripts": "0.0.38" loose the ability to show the ts file into browser. we could not debug the code

danbucholtz · 2016-11-03T05:40:07Z

@jaspal27, install 0.0.39 which has this in place. We also have 0.0.40 coming out in the coming days that has even "better" source maps.

Thanks
Dan

jaspal27 · 2016-11-03T13:23:40Z

Thanks it is working with me.

On Wed, Nov 2, 2016 at 10:40 PM, Dan Bucholtz notifications@github.com
wrote:

@jaspal27 https://github.com/jaspal27, install 0.0.39 which has this in
place. We also have 0.0.40 coming out in the coming days that has even
"better" source maps.

Thanks
Dan

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#158 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ACFYgMhe59CuqKRMqXTxTqRuJGSzZzOVks5q6XO5gaJpZM4KWsz-
.

Thanks,
Jaspal Singh

danbucholtz added 3 commits October 14, 2016 02:04

feature(webpack): webpack bundling support

67a007d

chore(bundle): disable webpack by default

07e16ce

merge commit

4168a2f

marcoturi mentioned this pull request Oct 14, 2016

Incorrect Coverage marcoturi/ionic-boilerplate#5

Closed

adamdbradley reviewed Oct 14, 2016

View reviewed changes

src/bundle.ts

.catch( (err: Error) => {

throw new BuildError(err);

});

} else {

Copy link

Contributor

adamdbradley Oct 14, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No else

adamdbradley reviewed Oct 14, 2016

View reviewed changes

src/bundle.ts

});

} else {

return rollupUpdate(event, path, context)

.catch( (err: Error) => {

Copy link

Contributor

adamdbradley Oct 14, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.catch(err =>

adamdbradley reviewed Oct 14, 2016

View reviewed changes

src/bundle.ts

return bundleConfig;

}

const taskInfo: TaskInfo = {

Copy link

Contributor

adamdbradley Oct 14, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's not create a new bundle config

adamdbradley reviewed Oct 14, 2016

View reviewed changes

adamdbradley and others added 7 commits October 15, 2016 11:24

feat(webpack): default webpack, set bundler via configs

09bc511

chore: remove old comments

f4d7a24

fix(ngc): use config values in the generated tsconfig

e1fc8d5

merge

fbeb044

Merge branch 'master' into feature/webpack

52b6f06

merge master

0d162b2

fix(webpack): use context when resource is not found

e925362

danbucholtz commented Oct 17, 2016

View reviewed changes

chore(version): ticking patch version

f9b8247

danbucholtz merged commit 41dc3a3 into master Oct 17, 2016

danbucholtz deleted the feature/webpack branch October 17, 2016 21:37

timofeysie mentioned this pull request Dec 21, 2016

uglifyjs failed: SyntaxError: Unexpected token: name (check) calvinmetcalf/crypto-pouch#45

Closed


		const transpiledFiles = getCachedTranspiledTsFiles();

		module.exports = function ionicLoader(source: any, map: any) {

		}


		export function createWebpackBundle(context: BuildContext, configFile: string): Promise<any> {

feature(webpack): webpack bundling support #158

feature(webpack): webpack bundling support #158

Uh oh!

Conversation

danbucholtz commented Oct 14, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TheLarkInn commented Oct 14, 2016

Uh oh!

rkostrzewski commented Oct 15, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marsanla commented Oct 19, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

danbucholtz commented Oct 19, 2016

Uh oh!

mehrshadpezeshk commented Oct 21, 2016

Uh oh!

danbucholtz commented Oct 21, 2016

Uh oh!

mehrshadpezeshk commented Oct 21, 2016

Uh oh!

tonyawad88 commented Oct 22, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

luchillo17 commented Oct 22, 2016

Uh oh!

tonyawad88 commented Oct 22, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

luchillo17 commented Oct 22, 2016

Uh oh!

tonyawad88 commented Oct 22, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

luchillo17 commented Oct 22, 2016

Uh oh!

luchillo17 commented Oct 22, 2016

Uh oh!

jaspal27 commented Nov 3, 2016

Uh oh!

danbucholtz commented Nov 3, 2016

Uh oh!

jaspal27 commented Nov 3, 2016

Uh oh!

Uh oh!

marsanla commented Oct 19, 2016 •

edited

Loading

tonyawad88 commented Oct 22, 2016 •

edited

Loading

tonyawad88 commented Oct 22, 2016 •

edited

Loading

tonyawad88 commented Oct 22, 2016 •

edited

Loading