Skip to content
This repository has been archived by the owner on Jul 12, 2018. It is now read-only.

Update to webpack 0.9.x #144

Closed
meaku opened this issue Jan 7, 2013 · 44 comments
Closed

Update to webpack 0.9.x #144

meaku opened this issue Jan 7, 2013 · 44 comments
Assignees

Comments

@meaku
Copy link
Member

meaku commented Jan 7, 2013

We should update to the current version of webpack before release v1.
Maybe @sokra could help us to improve the whole bundle thing?

@ghost ghost assigned meaku Jan 7, 2013
@sokra
Copy link
Contributor

sokra commented Jan 7, 2013

👍

@meaku
Copy link
Member Author

meaku commented Jan 10, 2013

@jhnns told me that there will be breaking changes in 0.9, so we will wait until it's released and skip 0.8.
@Soka when do you think 0.9 will be released?

@sokra
Copy link
Contributor

sokra commented Jan 10, 2013

I have too little time for 0.9 currently, so it will take a month until beta at minimum.

You should base on 0.8, which is prettly solid and the breaking changes for 0.8 are mostly option names and internal refactoring.

Here is a changelog with changes from 0.6 to 0.8. I think you only have to change some of the passed options and some calls to loaders. I. e. bundle/lazy!file changes to bundle?lazy!file.

Additionally to the migration I would refactor the following (optional):

  • remove generateTmpModules and use loaders for renderFillXXX and renderBootstrapClient.
  • then use webpack-dev-server or webpack-dev-middleware for development. (Caching ftw, speeds up development)

Maybe some more loaders are interesting for you, i.e. jshint...

@jhnns
Copy link
Member

jhnns commented Jan 11, 2013

I agree on your suggestions for refactoring. generateTmpModules was only a makeshift solution because it was easier to grasp (and we had other issues). I've reworked nodeclass which is now called alamid-class. Since it uses prototypical inheritance and does not need any compilation for the client we're able to make the bundling easier and much more straightforward.

We're planing to provide all nice features of webpack (such as webpack-dev-server) under the alamid namespace. So for example if the developer types alamid start dev-server alamid calls the webpack-dev-server internally with all options that work well together with alamid. I'm really looking forward to webpack 0.9 😄

meaku added a commit that referenced this issue Jan 11, 2013
@sokra
Copy link
Contributor

sokra commented Jan 11, 2013

I'm looking forward to webpack 0.10, which has Hot Code Replacement.... Really want it 😃

meaku added a commit that referenced this issue Jan 12, 2013
- separated val-function and renderer to make thing clearer
- passing rootPath to template to resolve the right file
- related to #144
meaku added a commit that referenced this issue Jan 12, 2013
- some path-handling fun...
- tidy tidy
- bundle with val-loader is working now
- #144
meaku added a commit that referenced this issue Jan 12, 2013
@meaku
Copy link
Member Author

meaku commented Jan 12, 2013

My local build works, but the remote version fails... awesome! 🐒

meaku added a commit that referenced this issue Jan 12, 2013
- should fix the build #144
@meaku
Copy link
Member Author

meaku commented Jan 12, 2013

Fixed! The next step will be the dev-middleware.

@meaku
Copy link
Member Author

meaku commented Jan 14, 2013

I implemented the webpack-middleware to handle static request in dev-mode.
Unfortunately the rebundling is kind of a problem...

I seems like the webpack-middleware is always serving cached files if already compiled and only recompiles in watch-mode. Watch seems like a good solution but as we experienced with nof5 it has troubles to watch many files and fails on some os'es. I wasn't able to test the watch mode, because it had to watch to many files and osx restricts the watchers to a relatively low number. Of course there are workarounds by setting a different number on os-level but i would prefer to have a second option for cache invalidation.

Maybe webpack could rebundle on every request but load existing files that didn't change from cache instead of starting the whole rebundling process. Would there be a performance improvement or does the analyzing part take the most time?

What do you think @sokra?

PS: its not pushed yet, because i wanted to find a suitable solution before publishing to master.

@sokra
Copy link
Contributor

sokra commented Jan 14, 2013

I didn't know about this osx problem. On windows and linux I never had issues watching many files.

We could change the middleware so that watch: false bundle on every request. Due the cache option it would be faster... but I think you would notify the delay.

But this would fix the watch mode on OSX... (Sadly, watch mode is so cool)

We could reduce the number of watched files by merging them by directory. But this would result in false positives...

@sokra
Copy link
Contributor

sokra commented Jan 14, 2013

I didn't know about this problem with OSX. I never had problems on windows and linux.

We could change the behavior for the middleware on watch: false to rebundling on request. It would be faster because it would cache stuff, but there would be a delay on each request. (I think this is ok)

But what's the long term solution for watch mode on OSX? We could reduce the number of watched files by merging them by directory (false positives). How low is the limit?

@sokra
Copy link
Contributor

sokra commented Jan 14, 2013

ahrrg... github had update problems... and I retyped it.... :(

sokra added a commit to webpack/webpack-dev-middleware that referenced this issue Jan 14, 2013
@sokra
Copy link
Contributor

sokra commented Jan 14, 2013

You can try the new version of the middleware, which support watch = false.

@meaku
Copy link
Member Author

meaku commented Jan 14, 2013

Thanks @sokra! I tried the new version of the dev-middleware and it's simply awesome. Compared to our latest - rebundle the whole thing every reload approach - it is very very fast. I will cleanup the whole thing and commit&merge tomorrow.

But what's the long term solution for watch mode on OSX? We could reduce the number of watched files by merging them by directory (false positives). How low is the limit?

I will check on this one and find out about the limits. You suggest merging all files of a folder to reduce the number of files watched? Did i get that one right?

@sokra
Copy link
Contributor

sokra commented Jan 14, 2013

Nice... With watch mode it would be even faster, because it would start compiling after files save....

Yes you get it right. Maybe this could be a separate module multi-watcher or similar ;)

@jhnns
Copy link
Member

jhnns commented Jan 15, 2013

Compiling on request requires less performance but delays serving the bundle. And as a developer I would rather let my PC work harder than me wait longer 😀

@sokra How do you watch files? I'm using isaacs supervisor when I need to watch for file changes. But I haven't tried it with OSX either.

@meaku
Copy link
Member Author

meaku commented Jan 15, 2013

@jhnns: The re-bundling is really quite fast, even if not using watch. We could make it configurable to use ether watch or rebundling on every request. This should be part of the yet to implement Bundle.class.js.

@sokra
Copy link
Contributor

sokra commented Jan 15, 2013

@jhnns The standard fs.watch API.

I watch every single file your bundle depends on, not only the directory the programm is started from.

@meaku With watch mode you could automatically refresh the browser on change.

@meaku
Copy link
Member Author

meaku commented Jan 15, 2013

Ok dudes! Seems like watch is working and it failed because of some other weird circumstances during testing yesterday. 👍

At least with the todo-mvc-example it works nicely. It might break if the files being watched will be too many. Maybe we could add some kind of limit for the watch function, to fall back to re-bundling in case the limit is exceeded?

@meaku
Copy link
Member Author

meaku commented Jan 15, 2013

Links related to the limit:

My limit on osx is set to 256 as a default value. Dunno about linux/windows.
But it looks like watchand watchFile are using different functions internally, so the limit seems to be different.

meaku added a commit that referenced this issue Jan 15, 2013
- made bundle.js the main module for the alamid-bundle process
- using watch mode for webpack-dev-middleware
- passing the config to the bundle-functions in order to decouple
- using the latest development version of webpack-dev-middleware
@meaku
Copy link
Member Author

meaku commented Jan 15, 2013

I made alamid create a bundle in production mode, if there is no app.js in bundle folder. This way you could ether create your own bundle and ship it with your project, create it in init.server.js before starting the actual server or let it be created for you by alamid if you don't need any custom options.

@meaku
Copy link
Member Author

meaku commented Jan 15, 2013

There is one problem left with the whole bundle-thingy. I replaced most tmp-files with the val-loader, but the bootstrap still has to be created anyhow. Even if we would create a static empty bootstrap with a single val-require to the bootstrapRender-function we have to copy this file to the bundle-folder. One option would be to solve this via our alamid-cli but if someone doesn't want to use the cli, he/she has to create the file manually which is error prone.

This is kind of pain in the ass, because you can't simply bundle, but have to render/copy the bootstrap before bundling and delete it afterward. The bootstrap has to be re-generated if a new file was added to the project manually (View, Model, Schema, Service). We would need separate watchers for those folder in order to regenerate the bootstrap, which would cause the bundle to regenerate as well.

One options would be to get rid of the registries, which would mean more explicit definition by the developers but less hazzle for the bundling process. But we wanted to talk about this whole registry thing anyway.

For now i would document it as a limitation:

If you add new Models/Schema/Pages/Services, you will have to restart your server.

@sokra webpack doesn't cache val-loaders, right? So are the registries (solved via val-loaders) always reloaded if an existing file was changed? I.e. if change a template .html and the rebundling causes the val-loaders to rebundle?

We have to think of a way to generate and delete the bootstrap.js before/after the bundler starts. How about events like beforeBundle(next), afterBundle(next) which could be used by us to generate and delete the file for normal webpack bundling and the webpack-dev-middleware. Maybe it's just our special use-case and not a great generic solution for webpack. But i think pre and post events might be interesting for bundling-purposes anyway?

sokra added a commit to sokra/alamid that referenced this issue Jan 15, 2013
@sokra
Copy link
Contributor

sokra commented Jan 15, 2013

See pull request for solution.

The val-loader can be made cacheable by the cacheable query parameter. (val?cacheable!./some-file)

But you can use real loaders, so you don't have to use the val-loader.

The bootstrap.client.js can also replaced by a loader, so you don't have to generate the tmp module.

Loaders can access the webpackOptions, so you can pass any custom parameters, like the alamid config.

A loader can flag itself as cacheable, by this.cacheable().

@meaku
Copy link
Member Author

meaku commented Jan 15, 2013

Very nice! Thanks for your work. I will take a closer look at it and merge it 👍

@meaku
Copy link
Member Author

meaku commented Jan 15, 2013

I checked it locally and a single test, which was to check the content of the rendered bootstrap is failing due to the changed bootstrap content. I will fix this one on my machine and merge. It's really awesome to get rid of the bootstrap.tmp generation 👯.

meaku added a commit that referenced this issue Jan 15, 2013
- thanks again for your awesome webpack-module and your helping hand :)
- #144
@jhnns
Copy link
Member

jhnns commented Jan 15, 2013

That's why I want to get rid of those registries. They're making bundling more complex and bring caching issues. Every registry that can be replaced by an explicit require is better (although the developer has to write more).

@meaku
Copy link
Member Author

meaku commented Jan 24, 2013

@sokra are you going to publish the latest version of the webpack-dev-middleware?

@sokra
Copy link
Contributor

sokra commented Jan 24, 2013

Oh sorry, it's now published...

@meaku
Copy link
Member Author

meaku commented Jan 24, 2013

Thanks ;)

@meaku meaku closed this as completed Jan 24, 2013
@sokra
Copy link
Contributor

sokra commented Feb 1, 2013

webpack 0.9 will watch only 100 items. The total number is merged by directories ;)

This is the crazy hacked code doing this: lib/node/NodeWatchFileSystem (somewhere is a bug... 😄 need to revisit it before release)

@meaku
Copy link
Member Author

meaku commented Feb 1, 2013

Crazy 🐴. But you still have to take care of those files, and re-merge to a file? How do you do it ? Manually via timeouts?

@sokra
Copy link
Contributor

sokra commented Feb 1, 2013

an example if the limit would be 4 and we want to watch these files:

/home/user/proj/a
/home/user/proj/b
/home/user/proj/dir/c
/home/user/proj/dir/d
/home/user/proj/dir/sub/e

it would be merged to this:

/home/user/proj/a
/home/user/proj/b
/home/user/proj/dir
   /home/user/proj/dir/c
   /home/user/proj/dir/d
   /home/user/proj/dir/sub/e

and only

/home/user/proj/a
/home/user/proj/b
/home/user/proj/dir

is watched.

on a change on /home/user/proj/dir

it would read the timestamps for

/home/user/proj/dir/c
/home/user/proj/dir/d
/home/user/proj/dir/sub/e

to figure out what changed...

@meaku
Copy link
Member Author

meaku commented Feb 1, 2013

Ah! You're watching dirs instead of files?

@sokra
Copy link
Contributor

sokra commented Feb 1, 2013

yes, is this a problem?

@meaku
Copy link
Member Author

meaku commented Feb 1, 2013

Nope! I was just wondering. I thought you were merging separate files in a single file to reduce watched files.
Although i'm not sure if dir-watchers are as reliable as file-watchers. I think @topa had some problems with watchers some time ago. But let's see how it works :) The webpack-middleware is simply awesome btw. Makes development way faster 🚀

@jhnns
Copy link
Member

jhnns commented Feb 1, 2013

I'm really looking forward to! Don't want to affront you but the code looks like callback hell 😄

@sokra
Copy link
Contributor

sokra commented Feb 1, 2013

@jhnns I know... :( That why I said "crazy hacked code". I will definitly revisit that file 😄

@sokra
Copy link
Contributor

sokra commented Feb 4, 2013

It doesn't work :(

The gamebreaker is that fs.watch do not watch subdirectories...

I have to get a new algorithm

@sokra
Copy link
Contributor

sokra commented Feb 10, 2013

webpack/webpack@d5b50eb

@jhnns
Copy link
Member

jhnns commented Feb 11, 2013

Ah nice. If there are any problems again, you could also try mikeal/watch. It's working quite well for nof5.

@sokra
Copy link
Contributor

sokra commented Feb 12, 2013

Thanks for the offer, but webpack has some specific requirements to the watching process which couldn't be fullfilled by the libraries I investigated in.

  • Watch files and dirs (including subdirs/files).
  • Watcher is applied a while after the file is read, so changes must be detected retrospectively.
  • Many files/dirs are watched and as less as possible watchers should be installed.
  • Files can be anywhere on the filesystem, not just in one directory.
  • Work on windows and *nix

mikeal/watch

  • may be used to watch files and dirs
  • cannot detect changes retrospectively
  • watch every file, do not merge by directory
  • watch only files in a specify directory
  • uses fs.watchFile which do not work on windows

@jhnns
Copy link
Member

jhnns commented Feb 12, 2013

Ok. But mikeal/watch works on windows 😄

@sokra
Copy link
Contributor

sokra commented Feb 12, 2013

ok it works... but really slow...

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

No branches or pull requests

3 participants