When promisifying, store does not preserve disableNunjucks property

[tcr]

This was causing difficulties in a less-maintained renderer I was using, but I think this may also count as a bug upstream.

[coveralls]

Coverage increased (+0.003%) to 97.155% when pulling 4966094 on tcr:patch-1 into e2b81fa on hexojs:master.

[JLHwung]

Hmm, I don't think Promise.promisify will discard property of a given nodeFunction. You can check on the Chrome console of http://bluebirdjs.com/docs/getting-started.html.

f = function() {}
f.foo = "bar"
// returns "bar"
Promise.promisify(f).foo 

[tcr]

You're correct! It's only Promise.method that doesn't preserve the property.

[coveralls]

Coverage decreased (-0.04%) to 98.706% when pulling 29ab720 on tcr:patch-1 into 58a8f8c on hexojs:master.

[NoahDragon]

@tcr Is that possible to pass in all the configuration to the sync process? If later on, new configuration needed/added, our goal is to not modify the same piece of code again.

[tcr]

@NoahDragon Would that be a proposal that instead of attaching disableNunjucks to the rendering function, it's passed in as an extra argument (or something) to the register method?

One way I can think of is to change sync to check if it's a boolean (former behavior), and if it's an object e.g. {sync: true, disableNunjucks: true} treat it as an options object. Or, just add another argument to the render method to accept an object hash.

The change would impact #2593 in either case—let me know if you think one of these is a good idea. cc @be5invis

[NoahDragon]

@tcr Sorry for the late reply. I would prefer the old approach, which check sync if it's boolean. We could review later how much impacts to the #2593 change.

[be5invis]

@NoahDragon
Whether enabling Nunjucks could be a "OR" of multiple sources:

• Whether user specifies to disable it in the entire site
• Whether user specifies to disable it in for a specific post
• Whether the renderer said that it can handle everything on its own

[JLHwung]

I raised an issue (petkaantonov/bluebird#1484) to request retaining custom properties on Promise.method. Before the decision is made, hexo is not looking forward to implement it is own copyDescriptors.

[stale]

This issue has been automatically marked as stale because lack of recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[github-actions]

How to test

git clone -b patch-1 https://github.com/tcr/hexo.git
cd hexo
npm install
npm test

[stevenjoezhang]

👍

[SukkaW]

Since Bluebird is no longer actively maintained, I guess we can accept this workaround for now.

We could, however, replace Bluebird.method with Bluebird.promisfy in the future.

[stevenjoezhang]

Found another patch: #4498
We could revert #4498 since the solution in this PR is simpler
See also #3860