browserify / commonjs-assert

Node.js's require('assert') for all engines
MIT License
295 stars 58 forks source link

have to modify util path because it was trying to ref my util folder #52

Closed chrisnorristech closed 3 years ago

chrisnorristech commented 3 years ago

Hi! ๐Ÿ‘‹

Firstly, thanks for your work on this project! ๐Ÿ™‚

Today I used patch-package to patch assert@2.0.0 for the project I'm working on.

Here is the diff that solved my problem:

diff --git a/node_modules/assert/build/assert.js b/node_modules/assert/build/assert.js
index a4bc0a8..69d87ed 100644
--- a/node_modules/assert/build/assert.js
+++ b/node_modules/assert/build/assert.js
@@ -35,10 +35,10 @@ var _require = require('./internal/errors'),

 var AssertionError = require('./internal/assert/assertion_error');

-var _require2 = require('util/'),
+var _require2 = require('../../util/'),
     inspect = _require2.inspect;

-var _require$types = require('util/').types,
+var _require$types = require('../../util/').types,
     isPromise = _require$types.isPromise,
     isRegExp = _require$types.isRegExp;

diff --git a/node_modules/assert/build/internal/assert/assertion_error.js b/node_modules/assert/build/internal/assert/assertion_error.js
index 9295f0a..df8c0b9 100644
--- a/node_modules/assert/build/internal/assert/assertion_error.js
+++ b/node_modules/assert/build/internal/assert/assertion_error.js
@@ -32,7 +32,7 @@ function _getPrototypeOf(o) { _getPrototypeOf = Object.setPrototypeOf ? Object.g

 function _typeof(obj) { if (typeof Symbol === "function" && typeof Symbol.iterator === "symbol") { _typeof = function _typeof(obj) { return typeof obj; }; } else { _typeof = function _typeof(obj) { return obj && typeof Symbol === "function" && obj.constructor === Symbol && obj !== Symbol.prototype ? "symbol" : typeof obj; }; } return _typeof(obj); }

-var _require = require('util/'),
+var _require = require('../../../../util/'),
     inspect = _require.inspect;

 var _require2 = require('../errors'),
diff --git a/node_modules/assert/build/internal/errors.js b/node_modules/assert/build/internal/errors.js
index 3f4350a..b9d7729 100644
--- a/node_modules/assert/build/internal/errors.js
+++ b/node_modules/assert/build/internal/errors.js
@@ -142,7 +142,7 @@ createErrorType('ERR_INVALID_ARG_TYPE', function (name, expected, actual) {
 }, TypeError);
 createErrorType('ERR_INVALID_ARG_VALUE', function (name, value) {
   var reason = arguments.length > 2 && arguments[2] !== undefined ? arguments[2] : 'is invalid';
-  if (util === undefined) util = require('util/');
+  if (util === undefined) util = require('../../../util/');
   var inspected = util.inspect(value);

   if (inspected.length > 128) {
diff --git a/node_modules/assert/build/internal/util/comparisons.js b/node_modules/assert/build/internal/util/comparisons.js
index 31d43ff..e1f4ad3 100644
--- a/node_modules/assert/build/internal/util/comparisons.js
+++ b/node_modules/assert/build/internal/util/comparisons.js
@@ -44,7 +44,7 @@ var hasOwnProperty = uncurryThis(Object.prototype.hasOwnProperty);
 var propertyIsEnumerable = uncurryThis(Object.prototype.propertyIsEnumerable);
 var objectToString = uncurryThis(Object.prototype.toString);

-var _require$types = require('util/').types,
+var _require$types = require('../../../../util/').types,
     isAnyArrayBuffer = _require$types.isAnyArrayBuffer,
     isArrayBufferView = _require$types.isArrayBufferView,
     isDate = _require$types.isDate,

This issue body was partially generated by patch-package.

ljharb commented 3 years ago

You shouldn't need all those ../ at all - can you elaborate on what led you to think you do?

chrisnorristech commented 3 years ago

Absolutely. I have a util folder in my code. For some reason, I kept getting the error:

"Unable to resolve "../../../app/util" from "node_modules/assert/build/assert.jsโ€

When I used expo-notifications

So once I modified the paths for the util call, it all works now.

On May 11, 2021, at 2:06 PM, Jordan Harband @.***> wrote:

You shouldn't need all those ../ at all - can you elaborate on what led you to think you do?

โ€” You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/browserify/commonjs-assert/issues/52#issuecomment-839015363, or unsubscribe https://github.com/notifications/unsubscribe-auth/AMVMESMNUF7TPANBRXRYQ63TNF53NANCNFSM44WRYDIA.

ljharb commented 3 years ago

That sounds like an expo bug, perhaps? (with bare specifier package names with a trailing slash, perhaps?)

util/ is a bare specifier, and should always grab the closest node_modules/util/ package.

chrisnorristech commented 3 years ago

That makes sense. It might be something with expo. I build a base project with the code from expo-notifications and had no issues. Its just with my project, and I have a util folder.

On May 11, 2021, at 2:09 PM, Jordan Harband @.***> wrote:

That sounds like an expo bug, perhaps?

util/ is a bare specifier, and should always grab the closest node_modules/util/ package.

โ€” You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/browserify/commonjs-assert/issues/52#issuecomment-839020248, or unsubscribe https://github.com/notifications/unsubscribe-auth/AMVMESMYCWVRSBKKLLIQKS3TNF6IBANCNFSM44WRYDIA.

ljharb commented 3 years ago

It'd be great if you filed that issue with them (assuming renaming your "util" to something else fixes the problem), since "needing to patch a package" is a very dangerous situation for any project to be in.

chrisnorristech commented 3 years ago

Or I guess I could rename my folder to utils :)

On May 11, 2021, at 2:14 PM, Jordan Harband @.***> wrote:

It'd be great if you filed that issue with them (assuming renaming your "util" to something else fixes the problem), since "needing to patch a package" is a very dangerous situation for any project to be in.

โ€” You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/browserify/commonjs-assert/issues/52#issuecomment-839026473, or unsubscribe https://github.com/notifications/unsubscribe-auth/AMVMESPIBV7OID3X5BN3T7TTNF6YRANCNFSM44WRYDIA.

chrisnorristech commented 3 years ago

do i just close this issue?

ljharb commented 3 years ago

Sure, let's do that :-) but i'd still request you file an issue with expo so they can fix the bug.

chrisnorristech commented 3 years ago

How do I do that?

On May 11, 2021, at 2:14 PM, Jordan Harband @.***> wrote:

It'd be great if you filed that issue with them (assuming renaming your "util" to something else fixes the problem), since "needing to patch a package" is a very dangerous situation for any project to be in.

โ€” You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/browserify/commonjs-assert/issues/52#issuecomment-839026473, or unsubscribe https://github.com/notifications/unsubscribe-auth/AMVMESPIBV7OID3X5BN3T7TTNF6YRANCNFSM44WRYDIA.

ljharb commented 3 years ago

Locate their repo, and do what you did here :-) I don't use expo myself, so I'm not sure which repo would be appropriate.