-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Package.json can't be found bugfix #170
Package.json can't be found bugfix #170
Conversation
src/Bundler.js
Outdated
} | ||
} | ||
} catch (err) { | ||
let currPath = Path.dirname(this.mainFile); | ||
currPath = currPath.substring(0, currPath.lastIndexOf('/')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May be more robust to use Path
to traverse up a directory.
currPath = Path.join(currPath, '..');
let deps = Object.assign({}, pkg.dependencies, pkg.devDependencies); | ||
for (let dep in deps) { | ||
if (dep.startsWith('parcel-plugin-')) { | ||
localRequire(dep, this.mainFile)(this); | ||
localRequire(dep, location)(this); | ||
} | ||
} | ||
} catch (err) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is err
always because it isn't found, or could it be something like access denied? Wouldn't want to keep traversing if it is found but just couldn't read it. That's a legitimate error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll look into it
@DeMoorJasper nice work! Do you think it is possible to add tests for this issue? In order to avoid future regressions? |
@marcioj i'll look into it, not sure how to write tests for this though |
src/Bundler.js
Outdated
} | ||
} | ||
} catch (err) { | ||
if (err.code === 'MODULE_NOT_FOUND' && location.length > 1) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we probably only want to do this if the require of package.json
failed, not one of plugins.
what if we used the config.load
util function that already exists to load the package.json file instead? that will already do the traversing for us.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ow totally looked over the localRequire of plugins being in the same try-catch.
@DeMoorJasper I'd try to create something based on this test. You can then copy the |
Tests should now handle parent folder package.json as well |
* add path traversing for package.json, plugin bug * improve * fix * add another test
* add path traversing for package.json, plugin bug * improve * fix * add another test
Currently the package.json file is only searched for within the directory and not in parent directories, this should fix this.
As discussed here: closes #167