fix: plugin no longer crashes in rollup only scenarios

[xenobytezero]

Tasks

• I've read the CONTRIBUTING guide
• Necessary tests have been added

The plugin currently crashes when used in a Rollup only context.

The registerTools() function does a require.resolve at the top level, which will fail if the requested package does not exist. This commit reworks the registerTools function to wrap it in a try/catch.

While doing this, we also found that the code will not resolve packages like Rollup correctly. So this has also been fixed.

A require.resolve on rollup (tested with 4.14.3) will resolve to the rollup/dist/rollup.js path. The current code will then try and find a package.json at dist/package.json which will fail and so not include the package.

I attempted to add a rollup fixture for the testing purposes, but the current structure of the tests means that the code will traverse all the way up to the workspace root from the fixture folders and find both rollup and vite, regardless of which fixture is being used. I have the fixture locally but have not included it in this PR to reduce complexity.

[janbiasi]

Hey @xenobytezero! Thanks a lot for the various fixes and adjustments, appreciate it 😃

You've mentioned that you prepared a fixture for the test, but it will always resolve the root packages, right? We could disable hoisting in general as we only have one single production package in the "monorepo" itself.

Let me know what you think :)

[xenobytezero]

Hey @xenobytezero! Thanks a lot for the various fixes and adjustments, appreciate it 😃

You've mentioned that you prepared a fixture for the test, but it will always resolve the root packages, right? We could disable hoisting in general as we only have one single production package in the "monorepo" itself.

I had thought about this, but disabling the hoisting wont on it's own help.

In a Rollup scenario, Vite is still as a direct dependency in the root workspace, and the request.resolve() code will continue to travel all the way up regardless. The code could maybe be changed to use the getCorrespondingPackageFromModuleId code which could be set to a very specific traversal limit for the tests, but that seems a bit brittle and doesnt actually work for all scenarios (Vite would need a max traversal of 1, Rollup needs 2 because of the mentioned export map problem). It's doable, but I feel there has to be a better solution out there. If I come up with anything I'll let you know.

[xenobytezero]

Apologies for the extra commits, had to do some author fixing.

[xenobytezero]

I also added some additional filtering to the moduleParsed analysis to ignore Rollup virtual modules (where the IDs start with \0) as I noticed a lot of resolutions that were traversing all the way up the directory tree because they couldn't resolve a package.json due to the strange character at the start of the path. All the tests continue to pass.

[janbiasi]

@all-contributors please add @xenobytezero for bug reports and code

[allcontributors]

@janbiasi

I've put up a pull request to add @xenobytezero! 🎉

[janbiasi]

@xenobytezero thanks a lot for your efforts on the bugs! I'm now fine merging it even without tests as it seems quite hard to implemented (did some fiddling tonight but couldn't find a way to declare sufficient test cases neither). I'd be fine moving forward and merging your PR - is everything included from your side?

I will also merge #70 after this PR got merged.

[xenobytezero]

Yep, that's now everything from my side. Happy to have the PR merged.

[github-actions]

🎉 This PR is included in version 1.1.1 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀