search

Macil / flow-copy-source

Script to copy javascript files and append ".flow" to the filename
MIT License
173 stars 13 forks source link

Make chokidar an optional dependency #29

Closed jedwards1211 closed 4 years ago

jedwards1211 commented 4 years ago

I guess flow-copy-source has some watch commands, but I don't use them, and its old version of chokidar depends on an old version of fsevents which doesn't build on Node 12.

Macil commented 4 years ago

Fsevents itself is an optional dependency of chokidar, so there shouldn't be an issue. The fsevents dependency can fail to install (and will fail on any non-MacOS platform) without causing the install of flow-copy-source or chokidar to fail. Please let me know if you see flow-copy-source failing to install on any current version of Node. I believe you're seeing an error message from fsevents failing to install (which is expected on non-MacOS platforms) but that error message should be non-fatal. Moving chokidar to be an optional dependency wouldn't cause that error message to go away, because it would still be attempted to be installed. (If you think the error message should be quieter or more specific, then I think that should be on npm/yarn or the fsevents package to address.)

If it can be avoided, I'd rather not have any functionality gated on an optional dependency. If an optional dependency was involved, I'd want it wrapped in something that could provide fallback support to give the functionality when the optional dependency wasn't there. Chokidar is itself that wrapper that provides fallback support in case fsevents isn't installed.

(The current version of flow-copy-source depends on the latest version of chokidar which optionally depends on the latest version of fsevents, though fsevents only compiles on MacOS.)

jedwards1211 commented 4 years ago

I see. Unfortunately it seems like ignoring optional dependencies with yarn isn't very convenient. In any case I'm out of luck until Babel 8 is released, because @babel/cli 7 depends on the old fsevents.