search

alexgorbatchev / gulp-print

This is a very basic gulp plugin that prints names of files.
MIT License
31 stars 11 forks source link

print is not a function #20

Open aliaksandr-master opened 6 years ago

aliaksandr-master commented 6 years ago

I noticed that you compiled your module by bablel like (tool from ES2016 to JS).

But for nodejs you need to export module.exports = print not exports.default = print

You can solve this by compile it with property commonJS modules or use only nodejs 8.x API

But right now I need to rollback to v3 of your package. sadness :^(

alexgorbatchev commented 6 years ago

Tried making it work both ways and couldn't figure out how. I think it also makes sense at this point to move to ES modules as primary. With that in mind, old require('gulp-print') becomes require('gulp-print').default

const gulp = require('gulp');
const print = require('gulp-print').default;

What do you think?

aliaksandr-master commented 6 years ago

I think you should support LTS version of nodejs as minimum. And should use the agreement in this infrastructure

I don't want to look as rude, but I use It for me =)

hoschi commented 6 years ago

https://rollupjs.org/guide/en is normally for libraries where webpack is for web to come over this problem. Don't want do be rude aswell, but it breaks also the support from gulp infrastructure like gulp-load-plugins.

alexgorbatchev commented 6 years ago

Please help me understand what is currently broken? Is require().default breaking something else?

hoschi commented 6 years ago

I work on a PR, more tomorrow ;)

Alex Gorbatchev notifications@github.com schrieb am Mi., 14. Feb. 2018, 20:19:

Please help me understand what is currently broken? Is require().default breaking something else?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/alexgorbatchev/gulp-print/issues/20#issuecomment-365715640, or mute the thread https://github.com/notifications/unsubscribe-auth/AAJ9OOOYOkAiL-OJQTI2IWDfw98-hmyLks5tUzHPgaJpZM4R5Gc4 .

hoschi commented 6 years ago

You using TypeScript which is not supported by the node environment, which is fine in general. Problem is "just" that you exported the functionality in away which is very unusual in that environment and by that it brakes the expectations of that environment. That was what @aliaksandr-master said earlier. Modules are expected for v10 LTS, see http://2ality.com/2017/09/native-esm-node.html#when-will-es-modules-be-available-without-the-command-line-option

I think it also makes sense at this point to move to ES modules as primary.

Node has no support for ES modules at the moment, so I don't know how this makes sense. If this lib would be something is usable on the web and with node it would make sense to me. Where/How do you use this lib that as a module instead of a library?

Tried making it work both ways and couldn't figure out how.

I helped you with that by created PR #22 to make it work in both envs, CJS and ES6 modules. By adding module property in your package.json, tools which work with ES6 modules pick the module and default behavior is a CJS as usual.

hoschi commented 6 years ago

@aliaksandr-master you can test the CJS version by changing your package.json to:

    "gulp-print": "hoschi/gulp-print#feature/rollup",