Fixed import errors on modules with shebang declarations #179
Conversation
As of NodeJS 12.16.0, importing modules with shebang declarations failed. This commit checks the first line for shebang declarations and removes them before trying to load the module.
|
I agree that this seemed to be triggered by Node.js - do you know what the change in node that triggered this? |
See the changes mentioned in the original issue: previously, Node would strip out the shebang before passing it on. Now it doesn't anymore. |
|
Any reason why this PR is not merged? This issue is a deal breaker when using this package on node versions >= 12.16 |
|
Same question as @fbilbie fully appreciate that there is an argument that the node.js change shouldn't have occurred. |
|
Hi, This would help us out with an issue as well. Can this be merged? Thanks! |
|
Thank you 🙏 Published as v6.0.0 🚀 |
Fixes #178
I feel like the issue should really be fixed in NodeJS, but this PR provides a workaround for the time being.
I've isolated the tests and the fix into two different commits, so that the fix commit could be reverted at a later date easily, should NodeJS provides a fix in the future.
This fixes the issue by removing the shebang declaration during the import via a regex that scans the first line of the module source. I believe this is a safe workaround because during a test environment, we don't need shebang declarations.
I've ran
npm testto ensure all other tests passed.Additionally I've tested my changes locally on https://github.com/apache/cordova-android who is exhibiting this issue on NodeJS 12.16.0+
Do note that this is not an issue on NodeJS 12.15 or earlier.