Handle non-exported package.json files

[jpaquim]

node's resolve throws an ERR_PACKAGE_PATH_NOT_EXPORTED error when trying
to resolve the package.json of a package which does not list itself in
its "exports" object.

This works around the issue by handling this case separately, using the
error message to get the path to the file, which is then required
correctly by the existing call to tryRequire().

In my case, this was happening with the uuid package, which has an "exports", but does not list the package.json itself in it, so node then complains.

I now see that the issue is already under discussion at #104, and in the uuid repo itself: uuidjs/uuid#444

Please let me know what you think of the proposed solution, I'm not sure if it's too workaround-ish...

[ctavan]

I don't think that parsing the error message is a good idea. I think the error message is nothing one could rely on, it could change at any point in time.

Instead, I believe that right now the plugin should probably just emit a warning if it cannot resolve the package.json so that package authors can start exporting it if necessary (it's not always necessary, in the case of uuid it is definitely not necessary for the svelte plugin to read uuid's package.json since it doesn't contain any svelte-specific information).

See: uuidjs/uuid#444 (comment)

We might even see Node.js change it's behavior again and always make package.json resolvable, see nodejs/node#33460

[ctavan]

@jpaquim would you mind trying out #109?

[jpaquim]

@ctavan thank you for the cleanup!

Your solution #109 is working correctly here, displaying the warning as expected.

Just a question, is the warning actionable in any way? I guess it doesn't really make sense to push maintainers for every package using "exports" to also export the package.json file, right?

[ctavan]

@jpaquim thanks for testing. You are right: Right now the svelte rollup plugin can't really know if a package author just didn't expose package.json by accident or deliberately.

In the case of uuid for example, we currently deliberately don't expose the package.json and there's also no svelte-specific config in it.

I'm not sure how to make this more actionable. You may want to watch nodejs/node#33460 where we are trying to fix the root cause of this whole issue rather than providing workarounds like #109.

[jpaquim]

@ctavan thanks for the pointer to the upstream issue.

I guess my main doubt right now is that, in the situation where a package deliberately does not expose the package.json (as is apparently the case with uuid), does it make sense to issue a warning on every svelte rollup build that uses that package?

Assuming that node itself doesn't change its behavior with regards to how require.resolve() behaves, I expect there to be a huge number of these warnings (as they have a tendency to grow over time), false positives if you will. Then again, not issuing the warning can lead to really hard to debug scenarios, I think it may be the best approach for now, and let's watch the upstream node issue.

Thanks for all your work looking into this 👍

[benmccann]

Thanks for proposing a fix for this! I'm going to close it in favor of #119, which I think we're leaning towards as a solution