Skip to content

Conversation

cjihrig
Copy link
Contributor

@cjihrig cjihrig commented Mar 20, 2015

a465840 added strict type checking for the methods in the path module. However, dirname(), basename(), and extname() actually had some undocumented uses
in the wild. This commit loosens the type checking on those methods. Fixes #1215.

R=@rvagg

a465840 added strict type
checking for the methods in the path module. However, dirname(),
basename(), and extname() actually had some undocumented uses
in the wild. This commit loosens the type checking on those
methods.
@brendanashworth
Copy link
Contributor

Should a deprecation message be added for these functions?

@rvagg
Copy link
Member

rvagg commented Mar 20, 2015

for 1.6.1 this lgtm, we could improve later

cjihrig added a commit that referenced this pull request Mar 20, 2015
a465840 added strict type
checking for the methods in the path module. However, dirname(),
basename(), and extname() actually had some undocumented uses
in the wild. This commit loosens the type checking on those
methods.

Fixes: #1215
PR-URL: #1216
Reviewed-By: Rod Vagg <[email protected]>
@cjihrig
Copy link
Contributor Author

cjihrig commented Mar 20, 2015

Landed in 8de78e4.

@cjihrig cjihrig closed this Mar 20, 2015
@cjihrig cjihrig deleted the 1215 branch March 20, 2015 01:12
@rvagg rvagg mentioned this pull request Mar 20, 2015
rvagg added a commit that referenced this pull request Mar 20, 2015
Notable Changes:

* path: New type-checking on path.resolve()
  <#1153> uncovered some edge-cases
  being relied upon in the wild, most notably path.dirname(undefined).
  Type-checking has been loosened for path.dirname(), path.basename(),
  and path.extname(), (Colin Ihrig)
  <#1216>.
* querystring: Internal optimizations in querystring.parse() and
  querystring.stringify() <#847>
  prevented Number literals from being properly converted via
  querystring.escape() <#1208>,
  exposing a blind-spot in the test suite. The bug and the tests have
  now been fixed (Jeremiah Senkpiel)
  <#1213>.
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.

3 participants