"TypeError: gen.next is not a function" - Metalsmith 2.5.0

[exortech]

Description
After upgrading to Metalsmith 2.5.0, we're now getting the error:

TypeError: gen.next is not a function

When metalsmith-watch is triggered in response to a file change.

Environment

• OS (run node -e "console.log(process.platform)"): MacOS
• Node|npm version (run node -v && npm -v): v16.15.0
  8.5.5
• Metalsmith version: (run npm view metalsmith version): 2.5.0

Reproducing the bug

Further debugging to follow.

[exortech]

Ok, it looks like the issue is that metalsmith-watch expects that metalsmith.read is a coroutine. Changing the method signature to return promises breaks the implementation for this plugin. As per the change in this commit: 16a91c5

[webketje]

Metalsmith < 2.5.0 generator functions were already not intended to be called with unyield in client-code (they could be called just like regular callbacks), so this would be an issue with metalsmith-watch. Metalsmith is not going back to generators, but if you would really like to keep using ms-watch (that I understand in the absence of another alternative), you could fork it and npm i github:exortech/metalsmith-watch, it's a 1-line change (line 179 in addition to removing the unyield dependency) from

unyield(metalsmith.read())((err, files) => { ... }

to (this was how metalsmith-watch should've always used it)

metalsmith.read((err, files) => { ... }

or

metalsmith.read()
  .then((files) => { ... })
  .catch(err => { ... })

As mentioned in the GH team I'm currently considering alternative watch implementations (in priority this is right behind a static method and a good js bundler plugin)

[exortech]

Thanks, Kevin. I'm fine with patching my fork of metalsmith-watch with this change. At some point, that project needs to be brought up-to-date... I understand not going back to generators - totally makes sense. However, I also wanted to make sure that you were aware that this change to Metalsmith core was a breaking change for plugins like metalsmith-watch. Regardless of whether it's an intended use of the method or not, the generator function was exposed in the interface. And this release does change the interface - which is not something that would normally be done in a minor release. I'm not sure that there's much that you can do about it at this point given that the release has already been published. But perhaps it's worthwhile to call this out more clearly in the Changelog - or link to this issue for other users of metalsmith-watch.

…

On Tue, Jun 14, 2022 at 1:32 AM Kevin Van Lierde ***@***.***> wrote: Metalsmith < 2.5.0 generator functions were already not intended to be called with unyield in client-code (they could be called just like regular callbacks), so this would be an issue with metalsmith-watch. Metalsmith is not going back to generators, but if you would really like to keep using ms-watch (that I understand in the absence of another alternative), you could fork it and npm i github:exortech/metalsmith-watch, it's a 1-line change (line 179 <https://github.com/FWeinb/metalsmith-watch/blob/master/src/index.es#L179> in addition to removing the unyield dependency) from unyield(metalsmith.read())((err, files) => { ... } to metalsmith.read((err, files) => { ... } or metalsmith.read() .then((files) => { ... }) .catch(err => { ... }) — Reply to this email directly, view it on GitHub <#374 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAERGUS62IF536GMNNYS6DVPA7QHANCNFSM5YVFBHNA> . You are receiving this because you authored the thread.Message ID: ***@***.***>

-- Owen Rogers | Exortech Consulting @exortech <https://twitter.com/exortech> | http://exortech.com/

[webketje]

Regardless of whether it's an intended use of the method or not, the generator function was exposed in the interface.

I know, for new "private" members I took care to use non-enumerable, unexported JS symbols and marked several as @package, as well as omitted them from the official API docs (readFile, read, write, writeFile) because users/ plugin authors shouldn't rely on them or be prepared to do frequent updates.

[...] which is not something that would normally be done in a minor release

I adhere to semver as much as possible, but there are 2 notable exceptions (which, as you say, would be better to document):

• I did drop major Node version support for EOL versions in minor releases (normally dropping a major Node version regardless of its maintenance status is a major version change).
• If a change represents a major improvement that is backwards-compatible with 99% of use cases in plugins that have been updated at least once in the past 5 years, I will consider it eligible for inclusion in a minor release (as I did now).

Plugin registry now includes a notice to discourage users from using severely outdated plugins (no updates in > 5 years).

I will close this issue after adding retroactively, as you suggested, an entry to the changelog, and perhaps an extra note in the plugin registry for metalsmith-watch.

[exortech]

Sounds good. Thanks, Kevin.

…

On Tue, Jun 14, 2022 at 3:44 PM Kevin Van Lierde ***@***.***> wrote: Regardless of whether it's an intended use of the method or not, the generator function was exposed in the interface. I know, for new "private" members I took care to use non-enumerable, unexported JS symbols and marked several as @Package, as well as omitted them from the official API docs (readFile, read, write, writeFile) because users/ plugin authors shouldn't rely on them or be prepared to do frequent updates. [...] which is not something that would normally be done in a minor release I adhere to semver as much as possible, but there are 2 notable exceptions (which, as you say, would be better to document): - I did drop major Node version support for EOL versions in *minor* releases (normally dropping a major Node version regardless of its maintenance status is a major version change). - If a change represents a major improvement that is backwards-compatible with 99% of use cases in plugins that have been updated at least once in the past 5 years, I will consider it eligible for inclusion in a minor release (as I did now). Plugin registry now includes a notice to discourage users from using severely outdated plugins (no updates in > 5 years). I will close this issue after adding retroactively, as you suggested, an entry to the changelog, and perhaps an extra note in the plugin registry for metalsmith-watch. — Reply to this email directly, view it on GitHub <#374 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAERGRVBMLY5FTB3CJRGGTVPEDM5ANCNFSM5YVFBHNA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

-- Owen Rogers | Exortech Consulting @exortech <https://twitter.com/exortech> | http://exortech.com/