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

module: remove require('.') with NODE_PATH compatibilty #1452

Closed
wants to merge 1 commit into from

Conversation

silverwind
Copy link
Contributor

This basically reverts #1363 and removes the hack in place for the unintended use case of require('.') used in conjunction with NODE_PATH.

I'd like to give users a few months to accustom to the deprecation, so it might not be a candidate for 2.0.0, but rather 3.0.0.

@silverwind silverwind added module Issues and PRs related to the module subsystem. semver-major PRs that contain breaking changes and should be released in the next major version. labels Apr 17, 2015
@domenic
Copy link
Contributor

domenic commented May 5, 2015

We should at the same time start emitting warnings whenever NODE_PATH is used.

@silverwind
Copy link
Contributor Author

So the consensus is to remove NODE_PATH entirely? What's the reasoning behind this?

@bnoordhuis
Copy link
Member

It's been deprecated for ages. Seeing how it negatively interacts with the module system, I don't think we want to carry it forward forever.

@rlidwka
Copy link
Contributor

rlidwka commented May 5, 2015

Honestly, I don't see any negative implications of the NODE_PATH itself. Yes, some people are misusing it, but it's not a good enough reason to remove this feature imho.

@silverwind
Copy link
Contributor Author

What are the alternatives to NODE_PATH? Absolute require paths?

@silverwind
Copy link
Contributor Author

Let's move the discussion to #1627

@Fishrock123
Copy link
Contributor

I suggest we move this to like 4.0.0, since 3.0.0 seems like it should be soon according to v8.

@silverwind
Copy link
Contributor Author

Yeah, if 3.0.0 lands in the next 2 months, I'll delay this (according to the 3 months policy suggested recently).

@Fishrock123
Copy link
Contributor

See #1735 - it should (ideally for v8?) land this week..

(according to the 3 months policy suggested recently).

Where was that suggested? That was the original suggestion but doesn't really work when v8 breaks every 6 weeks.

@silverwind
Copy link
Contributor Author

@Fishrock123 I was just lazily following #12, but I see, it's not decided yet.

@Fishrock123
Copy link
Contributor

Should this still land in 3.0.0, or is it better we target 4.0.0?

@silverwind silverwind modified the milestones: 4.0.0, 3.0.0 Aug 3, 2015
@silverwind
Copy link
Contributor Author

Moved to 4.0, given 3.0 is imminent.

@rvagg
Copy link
Member

rvagg commented Aug 24, 2015

@nodejs/collaborators thoughts on this for 4.0.0? I'm leaning towards punting till 5.0.0 to avoid some of the pain of users upgrading from 0.10 and 0.12 straight to 4. This is just another pain point they would encounter and it doesn't seem necessary to rip this band-aid off right now.

@thefourtheye
Copy link
Contributor

@rvagg You are correct. I think we can defer all the current semver-majors to 5.0.0.

@rvagg rvagg modified the milestones: 5.0.0, 4.0.0 Aug 24, 2015
@mscdex
Copy link
Contributor

mscdex commented Aug 24, 2015

Moving it to 5.0.0 is fine with me.

@rvagg
Copy link
Member

rvagg commented Aug 24, 2015

Moving to 5.0.0, feel free to disagree though

@silverwind
Copy link
Contributor Author

👍 this way, at least the deprecation message will be visible for some time to new-time users.

@targos
Copy link
Member

targos commented Oct 9, 2015

ping @silverwind
5.0.0 is close. This has to be retargeted to master if you still want to land it.

@silverwind
Copy link
Contributor Author

5.0.0 is close

I thought we were doing a 4.2.0 next? Anyways, it's too early to land this.

@jasnell
Copy link
Member

jasnell commented Oct 9, 2015

4.2.0 and 5.0.0 are on independent parallel tracks. 4.2.0 is the LTS off
the current v4.x stable while 5.0.0 will be a new stable branch.
On Oct 9, 2015 2:55 PM, "silverwind" notifications@github.com wrote:

5.0.0 is close

I thought we were doing a 4.2.0 next? Anyways, it's too early to land this.


Reply to this email directly or view it on GitHub
#1452 (comment).

@silverwind silverwind modified the milestones: 6.0.0, 5.0.0 Oct 9, 2015
@silverwind
Copy link
Contributor Author

Alright, moving to 6.0.0 in this case.

@jasnell
Copy link
Member

jasnell commented Oct 9, 2015

Fwiw... The 6.0.0 stable will be cut right around early April 2016.
On Oct 9, 2015 2:58 PM, "silverwind" notifications@github.com wrote:

Alright, moving to 6.0.0 in this case.


Reply to this email directly or view it on GitHub
#1452 (comment).

@silverwind
Copy link
Contributor Author

Updated and targeted master: #3384

@silverwind silverwind closed this Oct 15, 2015
@silverwind silverwind removed this from the 6.0.0 milestone Oct 15, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module Issues and PRs related to the module subsystem. semver-major PRs that contain breaking changes and should be released in the next major version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.