Skip to content
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

Add coffee-script module as a dependency #68

Closed
ghost opened this issue Apr 8, 2017 · 25 comments
Closed

Add coffee-script module as a dependency #68

ghost opened this issue Apr 8, 2017 · 25 comments
Labels

Comments

@ghost
Copy link

ghost commented Apr 8, 2017

While running your module via code that is transformed by the webpack/babel, I get following issue:

Module not found: Error: Can't resolve 'coffee-script' in '/node_modules/vm2/lib'
 @ ./~/vm2/lib/main.js 19:10-34
 @ ./~/vm2/index.js
 @ ./lib/config.js
 @ ./lib/server.js

https://github.com/patriksimek/vm2/blob/master/lib/main.js#L19

In this line you're using coffee-script module, but it's not defined as a module dependency.
So when webpack tries to map all the requires it crashes cause it can't find the coffee-script module anywhere.

UPD: this is not a feature request, this is vm2 module dependencies issue. Until this module has this line present, it will not work with compiler option specified as coffee-script on any environment without coffee-script module installed.

@patriksimek
Copy link
Owner

Thank you for reporing an issue. I have almost no experiences with Webpack so propably I'm wrong, but what if you add coffee-script as a dependency in parent project? I'm assuming Webpack processes requires the same way Node does - it searches all node_modules folders up to the root.

@ghost
Copy link
Author

ghost commented Apr 25, 2017

I don't want to add a coffee-script as a dependency of my own project just because your module require it. I don't use coffee-script.

If you're using someone's module in your own project, you must declare it as a dependency.

@ghost
Copy link
Author

ghost commented Apr 25, 2017

Also if someone will install your module and that person would not have a coffee-script module installed anywhere (in project or global), it will also trigger that error. It doesn't depends on Webpack.

@patriksimek
Copy link
Owner

Ok I thought you're using coffee-script. In that case, this library is not webpack compatbile atm. I will try to figure this out if I have some free time.

@ghost
Copy link
Author

ghost commented Apr 26, 2017

This is not a case of compatibility with webpack.

It's a case of how you publish and supply your module to npm.

@patriksimek
Copy link
Owner

I don't agree, this library does work without coffee-script properly so there is no reason to link it as a required dependency. It is only used when user explcitly states that he or she wants to use coffee-script. I will refactor this to some kind of plugin mechanism.

@ghost
Copy link
Author

ghost commented Apr 26, 2017

The reason is that you explicitly require the coffee-script in your code, which assume it should be installed somehow.

If you already got the coffee-script installed on your machine – that's okay and will work, otherwise it will cause an issue when I've got no coffee-script installed anywhere and launched the vm with compiler option set to one of 'coffeescript', 'coffee-script', 'cs' or 'text/coffeescript' (as stated in https://github.com/patriksimek/vm2/blob/master/lib/main.js#L14-L18).

Also I don't agree that this is a feature request as your module doesn't work with coffee-script compiler if it wasn't already installed. And documentation say nothing about the requirement of coffee-script to be pre-installed.

@patriksimek
Copy link
Owner

patriksimek commented Apr 26, 2017

Yes, you have precisely described how it works right now. The library expects user to have coffee-script installed if the compiler is set to coffee.

@patriksimek
Copy link
Owner

You're right about the docs, it's updated.

@ghost
Copy link
Author

ghost commented Apr 26, 2017

Yeah, documentation fix doesn't solve the issue. You still have the require('coffee-script') line in your code, which will produce the error, at least for webpack.

@patriksimek
Copy link
Owner

Yes, and as I wrote in previous post, I will refactor this to some kind of plugin mechanism so it will be webpack compatible.

@ghost
Copy link
Author

ghost commented Apr 26, 2017

Never mind, if it's so hard for you to run one simple command:

> npm install coffee-script --save

I will no longer use nor wait for your project to fix the issue.

@ghost ghost closed this as completed Apr 26, 2017
@n-riesco
Copy link

@patriksimek Personally, I see the benefit of having some kind of plugin mechanism.

How about something simple like options.compiler being a function?

Then users that need CoffeeScript could set options.compiler to (code) => { return require('coffee-script').compile(code, {header: false, bare: true}); }.

@patriksimek
Copy link
Owner

patriksimek commented Apr 26, 2017

@dkfiresky Wow, you really expected me to force tens of thousands users to download coffee-script even if they don't need it just because you are not willing to wait for a proper solution? Thats sad.

@n-riesco Actually this is already supported. The problem is that dropping support for coffeescript option is a breaking change and I would like to try a better solution first (if there is any).

@ghost
Copy link
Author

ghost commented Apr 26, 2017

@patriksimek I expect you to deliver your module correctly. The use of coffee-script is your own responsibility. If you use it, you must define it as a dependency. If you don't agree, then remove that require statement and the compiler option that states coffee-script as an option.

This issue was created because your module doesn't work out of the box. Waiting 18 days for you to run a simple install command sounds like a joke.

@ghost
Copy link
Author

ghost commented Apr 26, 2017

Moreover if you state that your module compiles coffee-script, you must deliver that module as a dependency in your package.json.

@ghost
Copy link
Author

ghost commented Apr 26, 2017

you really expected me to force tens of thousands users to download coffee-script

That what you did by require('coffee-script') in your code. You expect me to install coffee-script even if I don't use it.

@ghost ghost reopened this Apr 26, 2017
@patriksimek
Copy link
Owner

Waiting 18 days for you to run a simple install command sounds like a joke.

You still expecting me to do what I said I'll never do. So again - this module is not compatbile with Webpack at the moment, please, don't use this library.

@ghost
Copy link
Author

ghost commented Apr 26, 2017

@patriksimek this module is not compatible nor runnable on any environment without coffee-script installed.

@patriksimek
Copy link
Owner

@dkfiresky Sure, if you think so. I'm no longer interested in convincing you that you're wrong. Can we please stop this useless discussion?

@ghost
Copy link
Author

ghost commented Apr 26, 2017

@patriksimek yes, sure. Please go read some npm docs.

@marktellez
Copy link

I get this too in a fresh react project, before I added vm2 as a dependency I had no errors about coffee-script

Failed to compile.

/Users/mark/node_modules/coffee-script/lib/coffee-script/coffee-script.js
Module not found: Can't resolve 'module' in '/Users/mark/node_modules/coffee-script/lib/coffee-script'

@guigrpa
Copy link

guigrpa commented Aug 28, 2018

Even though the tone of the discussion was not very constructive, I really believe coffee-script should be added as a dependency. It would be a very simple solution and would allow using this package in various environments (e.g. Webpack, Next, etc.).

@sreeram-dev
Copy link

This is now breaking proxy-agent and its dependent library degenerator.

@XmiliaH
Copy link
Collaborator

XmiliaH commented Jan 21, 2022

This needs to be configured in Webpack. The coffee-script module needs to be marked as extern. As far as I know there is no way to tell webpack via package.json or else that a module is an optional dependency.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants
@patriksimek @guigrpa @sreeram-dev @n-riesco @marktellez @XmiliaH and others