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

BUG: Package wrapper files generated by ServerlessEnterprisePlugin #68

Merged
merged 20 commits into from
Nov 21, 2019

Conversation

ryan-roemer
Copy link
Member

@alexdebrie -- I've got what I consider the "least bad" workaround of an awkward tension between SFE plugin generating assets last in line and then built-in packaging happening effectively first in line which means that a custom packager like Jetpack has no easy lifecycle hooks to get in and take over while still getting SFE generated files. If you or someone from the SFE team could review the comments and hack approach around the patchesForEnterprise function in index.js I'd super appreciate it! (Totally open to any other approach that would work, but this was unfortunately the best thing I could come up with and it's not great).

/cc @cbumstead

@alexdebrie
Copy link

@ryan-roemer Oof! Thanks for the heads up, and sorry we're making packaging difficult here. I'll check internally to see if anyone can take a look at this. Will get you an update one way or the other by mid-day tomorrow.

@medikoo
Copy link

medikoo commented Nov 21, 2019

@ryan-roemer if I understand correctly issue is that before:package:createDeploymentArtifacts hook as registered by Jetpack is invoked before one registered by @serverless/enterprise-plugin. If it'll be other way round there won't be an issue.

If that's the case, then I believe there's an easier workaround than one currently implemented in this PR (although still involves a hack):

Do not register for 'before:package:createDeploymentArtifacts' in plugin constructor. Instead register for an'initialize' event (it's run unconditionally, right before other hooks are processed).

Then, in 'initialize' listener, register callback for 'before:package:createDeploymentArtifacts' (at this point @serverless/enterprise-pluginlistener will already be there).

Of course, culprit is, that attaching a hook at this point requires a hacking approach as:

const hooks = serverless.pluginManager.hooks;
if (!hooks['before:package:createDeploymentArtifacts']) {
  hooks['before:package:createDeploymentArtifacts'] = [];
}
hooks['before:package:createDeploymentArtifacts'].push({
  pluginName: 'Jetpack',
  hook: hookCallbackGuaranteedToBeRunAsLast
});

That's probably best you can do now, and on our side, we'll think whether it's possible to introduce a non hacky way to that problem, at same time ensuring that @serverless/enterpise-plugin won't fail unexpectedly cause of some plugins.

@ryan-roemer
Copy link
Member Author

@medikoo -- Looks like it worked! Here's current implementation if you want to re-review: https://github.com/FormidableLabs/serverless-jetpack/pull/68/files#diff-168726dbe96b3ce427e7fedce31bb0bcR121

hook: delayedHooks[event]
});
});
},
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good 👍

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yay! Thanks for the review and suggestion!

@ryan-roemer ryan-roemer merged commit 855a554 into master Nov 21, 2019
@ryan-roemer ryan-roemer deleted the bug/dashboard branch November 21, 2019 20:56
@ryan-roemer
Copy link
Member Author

Released in [email protected]

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

Successfully merging this pull request may close these issues.

BUG: Serverless is no longer creating wrapper functions for serverless dashboard
3 participants