Skip to content
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

jspm bundle -wid with linked packages? #1687

Closed
slanted opened this issue Apr 5, 2016 · 16 comments
Closed

jspm bundle -wid with linked packages? #1687

slanted opened this issue Apr 5, 2016 · 16 comments

Comments

@slanted
Copy link

slanted commented Apr 5, 2016

Is there a way to use

jspm bundle src/app.js -wid

and have it work with linked packages as well?
It picks up everything in /src but not changes to the linked packages. I'm not using hot-reloading, just jspm link.

If it doesn't work right now, are there plans to support this, because that would be super cool.

@guybedford
Copy link
Member

Certainly we should look at supporting this too.

@oligot
Copy link

oligot commented Apr 7, 2016

According to this issue, sane doesn't support symlinks 😞
We should probably switch back to chokidar, which support symlinks.

oligot pushed a commit to oligot/jspm-cli that referenced this issue Apr 7, 2016
sane doesn't support symlinks[1].
Let's switch back to chokidar which supports symlinks.

Only tested on Linux: should probably test on Windows and OS X
to see if it works as expected.

[1] amasad/sane#42
@slanted
Copy link
Author

slanted commented Apr 9, 2016

Thanks a bunch for the hint @oligot . I've written a test on my own system that does something like this:

  1. When watch is indicated with -wid, go down the the jspm_packages/jspmRegistry folder (I'm assuming this might be a different folder name such as 'link' so it will detect) and list all files.
  2. Take any files from this listing that are soft links and add them to a 'toWatch' array, also add the lowestDir directory (src). I'm going to trim this down as much as I can so that the CPU is hit as little as possible.
  3. Use chokidar as you indicated instead of watchman and watch of the glob of files. I'd also like to trim down the watch to just certain extensions such as .js, .css, .json, .html or something like that to make it light.
  4. I noticed in the buildWatch callback function 'fn', that it went through the whole bundle() function again, including setting up another watch - I didn't think this was necessary and just let the events continue to do their work - so I enclosed the buildWatch function in an if (!watching) {} conditional and set watching=true at the end of the buildWatch function so it only gets called once. Seems like a waste of CPU to have the chokidar.watch command scan all of the directories again if the 'add', 'delete', and 'changed' events can listen for any changes - right? In my project I'm watching 4 different locally linked dependencies in jspm_packages and the scan can take a while. I think scanning just once and letting the events take care of the deltas is more efficient.

I can give someone the code, but I'm a github noob so I'm not sure how I'm supposed to do this.

Also I was wondering if the jspm bundle -wid command could be enhanced with something like

jspm bundle src/main.js -widl

where the 'l' is 'watch symlinked dependency src directories'. That way if a user only wants to watch the 'src' directory and nothing in the jspm_packages folder they could. But maybe that should be a function of the symlinks like I have it now. Just wondering if anyone would want that - maybe they want to do a rebundle if their locally linked dependencies ever change and only use watch for their 'src' dir changes.

Still cleaning this up a bit, but its working very well for me right now - other than my cpu still spins. Last question @guybedford : I did get this also working with watchman - even through the symlink. Want me to try and use that because of performance considerations? I'm not sure which watcher is better.

Just doing this for my development group - we have a rather large project we've written in jspm and we want a really smooth development workflow since we have about 10 developers sharing code - let me know if you'd like to see it or if you have other plans. For now I'm just sending the link.js file around and they are using it - I'll let you know how it goes.

@slanted
Copy link
Author

slanted commented Apr 9, 2016

Sorry my post was a little too quick - I'm going to try to get this working using watchman. Apparently its faster. If its not going to work through watchman - I'll dig around some more to see how the CPU load can be kept down with chokidar.

@slanted
Copy link
Author

slanted commented Apr 12, 2016

Well, I rewrote this using chokidar but I ran into windows 7 problems. So I rewrote it so it doesnt watch through the symlinks - but normalizes the paths by reading the symlink pointer. That seems to work - but now I want to reintegrate this back to using sane w/ watchman to see if that resolves it. Everything is working well on my mac, but I'm trying to find normalized node libraries that will work both with windows and macos/linux. For example when reading the soft link from windows I get a normalized path, but in macos its something like: ../../../../{dependency}

If someone else is working on this problem let me know - I don't want to double up efforts. Otherwise I'm going to keep going.

@guybedford
Copy link
Member

@slanted is it possible to just run the symlink target through path.resolve(symlinkTarget, symlinkPath) or similar?

@slanted
Copy link
Author

slanted commented Apr 25, 2016

@guybedford yes I'll try that - I'm just worried that the windows impl might be off, based on what I saw. I'm going to try making everything work with just watchman.

@jonricaurte
Copy link

@slanted Awesome to hear. Were you able to get somewhere with watchman? This would be a really useful feature.

@slanted
Copy link
Author

slanted commented May 19, 2016

@jonricaurte yes I've got it working and I'm using it. Let me know if you want to be a Guinea pig. :)

@jonricaurte
Copy link

jonricaurte commented May 25, 2016

@slanted got a little busy; sorry for the late response. Is it possible to make a pr and have @guybedford review it? @guybedford it seems like this ticket can be closed because this should be handled by symlinks in this ticket: #1789

@jonricaurte
Copy link

jonricaurte commented May 27, 2016

@guybedford is it possible to switch back to chokidar? Seems like the work is already done:

oligot@08058b9

Also, members at facebook admit chokidar has gotten way better than when they were looking into it while they were building sane (and sane seems to just be used as a fallback incase watchman is not installed rather than having good performance based on the comments in the link; they even mention dropping sane if chokidar had a watchman option):

facebook/react-native#628

I also made a ticket in chokidar's repo for adding a watchman option but based on the comment does not seem like it will be worked on unless someone submits a pr:

paulmillr/chokidar#486

What do you think?

Thanks.

@guybedford
Copy link
Member

I do like Watchman, and I like that it can fall back to native fs watching for small projects without needing to install a large c dependency.

Linking support shouldn't be too hard to implement I believe with a recursive stat stopping at jspm_packages package folders, which shouldn't be too performance expensive either.

@jonricaurte
Copy link

@guybedford Thanks for the clarification. This is now the only thing blocking us from switching to JSPM since we have many shared repos as dependencies for our main apps. Do you have a time frame on when you might be able to get around to this? Thanks!

Also, based on how good 0.17 is going you might want to make this the 1.0 of JSPM :)

@jonricaurte
Copy link

@guybedford Also, keep in mind that on chokidar's page it gives a glimpse to who is using it:

"Initially made for brunch (an ultra-swift web app build tool), it is now used in gulp, karma, PM2, browserify, webpack, BrowserSync, Microsoft's Visual Studio Code, and many others. "

Most likely, people who are using JSPM are also using browsersync, gulp, and karma so even though this would be adding a huge dependency to JSPM, most likely anybody using JSPM would already have it installed because the 3 repos listed above are using it. I know Watchman is very highly performant, but chokidar is basically right on it's heels. This feature would most likely be blocking many enterprise customers from using JSPM.

@jonricaurte
Copy link

@guybedford looks like watchman might be handling symlinks in the future:

facebook/watchman#105

Until then, someone made something using watchman that copies folders:

https://github.com/wix/wml

Just in case anybody wants symlinks.

@ffflabs
Copy link

ffflabs commented Jan 25, 2017

I have failed to find where is -wid mode explained in the docs. It's like, when I change any file in the bundle, the bundle is rebuilt?

If yes, then I would need to have 4 processes running, for my app uses an approach of a single entry point leading to 4 different packages, each of them with a bundle.

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

No branches or pull requests

5 participants