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

Add support for node 4.x.x #59

Closed
wmonk opened this issue Sep 9, 2015 · 18 comments
Closed

Add support for node 4.x.x #59

wmonk opened this issue Sep 9, 2015 · 18 comments

Comments

@wmonk
Copy link

wmonk commented Sep 9, 2015

Now that node 4.0.0 has been released as the stable version, it would be great to add support for this version.

@blond
Copy link
Contributor

blond commented Sep 9, 2015

👍

1 similar comment
@pezza3434
Copy link

👍

@tschaub
Copy link
Owner

tschaub commented Sep 9, 2015

I've started work on this. If anybody else wants to take it, feel free - I may not have a chance to finish it for the next week. It looks like this may require more significantly patching the built-in fs module and providing an implementation of binding.FSReqWrap.

@sleexyz
Copy link

sleexyz commented Sep 12, 2015

👍

2 similar comments
@mlegenhausen
Copy link
Contributor

+1

@kdelmonte
Copy link

+1

IgorMinar added a commit to IgorMinar/angular that referenced this issue Sep 26, 2015
mock-fs is currently incompatible with node 4.x, but a fix is in progress
tschaub/mock-fs#59

Since we are currently not actively developing the affected broccoli plugins,
the risk of disabling these tests is low, especially in the light of
improvements we get from node 4.x.
IgorMinar added a commit to IgorMinar/angular that referenced this issue Sep 26, 2015
mock-fs is currently incompatible with node 4.x, but a fix is in progress
tschaub/mock-fs#59

Since we are currently not actively developing the affected broccoli plugins,
the risk of disabling these tests is low, especially in the light of
improvements we get from node 4.x.
IgorMinar added a commit to IgorMinar/angular that referenced this issue Sep 26, 2015
mock-fs is currently incompatible with node 4.x, but a fix is in progress
tschaub/mock-fs#59

Since we are currently not actively developing the affected broccoli plugins,
the risk of disabling these tests is low, especially in the light of
improvements we get from node 4.x.
IgorMinar added a commit to IgorMinar/angular that referenced this issue Sep 30, 2015
mock-fs is currently incompatible with node 4.x, but a fix is in progress
tschaub/mock-fs#59

Since we are currently not actively developing the affected broccoli plugins,
the risk of disabling these tests is low, especially in the light of
improvements we get from node 4.x.
IgorMinar added a commit to IgorMinar/angular that referenced this issue Oct 3, 2015
mock-fs is currently incompatible with node 4.x, but a fix is in progress
tschaub/mock-fs#59

Since we are currently not actively developing the affected broccoli plugins,
the risk of disabling these tests is low, especially in the light of
improvements we get from node 4.x.
IgorMinar added a commit to IgorMinar/angular that referenced this issue Oct 3, 2015
mock-fs is currently incompatible with node 4.x, but a fix is in progress
tschaub/mock-fs#59

Since we are currently not actively developing the affected broccoli plugins,
the risk of disabling these tests is low, especially in the light of
improvements we get from node 4.x.
IgorMinar added a commit to IgorMinar/angular that referenced this issue Oct 3, 2015
mock-fs is currently incompatible with node 4.x, but a fix is in progress
tschaub/mock-fs#59

Since we are currently not actively developing the affected broccoli plugins,
the risk of disabling these tests is low, especially in the light of
improvements we get from node 4.x.
IgorMinar added a commit to angular/angular that referenced this issue Oct 3, 2015
mock-fs is currently incompatible with node 4.x, but a fix is in progress
tschaub/mock-fs#59

Since we are currently not actively developing the affected broccoli plugins,
the risk of disabling these tests is low, especially in the light of
improvements we get from node 4.x.
@ylallemant
Copy link

+1

1 similar comment
@luscus
Copy link

luscus commented Oct 4, 2015

+1

@ariporad
Copy link

ariporad commented Oct 5, 2015

+1, this is a really useful tool, and I'd love to be able to use it.

@sveisvei
Copy link

sveisvei commented Oct 5, 2015

+1

@ariporad
Copy link

ariporad commented Oct 5, 2015

@tschaub: Actually, how would I go about bringing support for this to node? I dislike +1ing without at least offering some help, and I'd really like to see this happen, so what can I do?

@catdad
Copy link

catdad commented Oct 6, 2015

@ariporad I started looking at this as well. The lib is laid out to mock each major version of Node separately. I added 4.x to the list to use the mock for 3.0.0, and all of the tests passed, so I am not actually sure what is wrong. It seems that, at least for most common things, the lib should already work. I will run my suite of tests tomorrow against this (I have a couple of projects that use this lib for testing) just to confirm.

Perhaps @tschaub can tell us a bit more about he problems he is aware of. I'd also like to help in getting this working.

@tschaub
Copy link
Owner

tschaub commented Oct 6, 2015

@catdad - if we want a quick win, we could use the monkey patched fs.js for version 3 when running Node 4. For that matter, the fs.js from version 0.8 could be used. This would only be a problem for people that are using features introduced between these versions.

I'm hoping to find a different way to swap out the fs binding in a way that would be more future-proof. But I'd also consider a pull-request that took the quick-win approach for v4 (bonus points if you can document any limitations or caveats associated with using fs.js from v3).

@voxpelli
Copy link

voxpelli commented Oct 6, 2015

As io.js v3 was more or less a release candidate for node.js v4 there shouldn't really be any differences and the CHANGELOG points out no changes in regards to the fs-module between the two: https://github.com/nodejs/node/blob/master/CHANGELOG.md#2015-09-08-version-400-stable-rvagg

The only fs-module change since io.js 3.3.0 that's in the CHANGELOG are two small changes in 4.1.0: https://github.com/nodejs/node/blob/master/CHANGELOG.md#2015-09-17-version-410-stable-fishrock123

I'm in favor of reusing the fs.js from version 3.

I think I'm actually in favor of always using the newest fs.js mock when a specific one can't be find for the current version of node – and just warn about the fact that some newer additions or tweaks might not fully match. That way we can discover shortcomings in newer versions and report those rather than having issues like this one that's about getting mock-fs to at all work.

@catdad
Copy link

catdad commented Oct 6, 2015

Looking at the links @voxpelli provided, and a bit of extra research, I also believe there is no differences to be concerned about between 3.x and 4.x (as long as the 3.x implementation here is good, of course).

I ran tests for all my projects using my updated mock-fs against 4.1.2, and all of my tests passed. I don't know about most people, but my projects tend to just use the read and write streams to read and write from the filesystem, which don't really change. It's not worth it to me to hold off on being able to test those simple cases because there might be an obscure change in a new release. If this was my lib, I'd go with @voxpelli 's approach of letting users report specific issues to new implementation of Node. This is actually the first module I have run across that does a hard coded version check.

@mlegenhausen
Copy link
Contributor

Would it be possible to mark the support for 4.x.x as experimental. I haven't encountered any bugs yet and it would be great to use the npm repository instead of a git branch.

tschaub added a commit that referenced this issue Oct 13, 2015
Add support for node v4.x

Fixes: #59
@danez
Copy link

danez commented Oct 14, 2015

Could we get a new release now please :)

@mlegenhausen
Copy link
Contributor

+1

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

No branches or pull requests