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

fix watching in node-loader #390

Merged
merged 1 commit into from Mar 10, 2015
Merged

fix watching in node-loader #390

merged 1 commit into from Mar 10, 2015

Conversation

jlongster
Copy link
Contributor

@SamyPesse The relative templates broke watching. In getSource the fullpath key in pathsToNames is now always fully absolute, but in the watcher the path it gets is still just a relative path. I think this fixes it, but can you verify?

Are you not using the watcher?

@@ -31,6 +31,7 @@ var FileSystemLoader = Loader.extend({
var watcher = chokidar.watch(p, { ignoreInitial: true });

watcher.on('all', function(event, fullname) {
fullname = path.join(process.cwd(), fullname);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not path.resolve ? It's using resolve in getSource

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That works, I forgot about that.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why not path.resolve(p, fullname) ?

@SamyPesse
Copy link
Contributor

I'm using the watcher but also the noCache option that's maybe why I didn't see this bug.

jlongster added a commit that referenced this pull request Mar 10, 2015
@jlongster jlongster merged commit 8974aa3 into master Mar 10, 2015
@jlongster
Copy link
Contributor Author

Yeah, noCache would still work because this is a bug in cache invalidation.

@@ -31,6 +31,7 @@ var FileSystemLoader = Loader.extend({
var watcher = chokidar.watch(p, { ignoreInitial: true });

watcher.on('all', function(event, fullname) {
fullname = path.resolve(fullname);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is fullname (from chokidar events) relative to p or absolute ?

Because if the path is relative, this will work only if p == process.cwd() which is not always the case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fullname is the path that changed according to the path you gave chokidar, p in this case. If p is absolute, fullname will be absolute (I think). If it's relative, it will be relative. If p is views, fullname will be views/index.html.

So basically this should work because chokidar's watch patch is relative to process.cwd(). If it isn't, you gave it an absolute path.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok :)

@jbmoelker jbmoelker deleted the fix-watching branch September 7, 2016 11:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants