Skip compile & packaging if --no-build is set

[jamesmbourne]

What did you implement:

Closes #398

Allow the --no-build flag to commands that invoke the before:package:createDeploymentArtifacts hook. This means that build output can be generated (e.g. in CI environments) and then deployed without having to re-build each time.

How did you implement it:

Use the existing build option to skip the compile and package steps for the before:package:createDeploymentArtifacts hook.

How can we verify it:

1. Generate build output with sls webpack
2. Run sls deploy --no-build
3. The existing .webpack (or other as specified by --out) directory should be used an no Webpack compilation occurs

Todos:

• Write tests
• Write documentation
• Fix linting errors
• Make sure code coverage hasn't dropped
• Provide verification config / commands / resources
• Enable "Allow edits from maintainers" for this PR
• Update the messages below

Is this ready for review?: YES
Is it a breaking change?: NO

[shredder2500]

I tested this today, everything is working as I would expect. what is left to do before this is ready for review?

[HyperBrain]

I will do a review this weekend and I've already tagged the related issue with 5.4.0 :-)

[hassankhan]

Hi @jamesmbourne, thanks for the PR 👍 ! Any chance you could add some documentation to the README.md as well?

[jamesmbourne]

Hey @hassankhan yes I will make sure to update that.

Testing it locally with a real deployment I have noticed some issues where it seems to bundle node-modules in the ZIP when running serverless deploy --no-build.

This appears to call the packaging phase - is there something that will need to be modified within that as well to be aware of the --no-build flag?

[shredder2500]

any updates on this?

[jamesmbourne]

@shredder2500 I've been a bit busy recently but I'll have another look at it and try and get the PR read for review!

[jamesmbourne]

@HyperBrain I've done some more digging at it seems like we will need to only skip the webpack:compile phase, but continue to run webpack:package to actually generate the ZIP containing the bundled JS.

In the webpack:package phase, the stats object from compilation is used to determine what should be deployed e.g:

const stats = {
  stats: [
    {
      compilation: {
        compiler: {
          outputPath: "/my/Service/Path/.webpack/service"
        }
      }
    }
  ]
};

If compile is skipped, we do not have this object to determine what has already been built. Perhaps we can use the information provided by this.webpackConfig instead? It contains the path to each built function (as needed here: https://github.com/serverless-heaven/serverless-webpack/blob/master/lib/packageModules.js#L86), although other usages of it (e.g. https://github.com/serverless-heaven/serverless-webpack/blob/master/lib/packExternalModules.js#L156) need access to more in-depth information about the compilation process.

Do you have any thoughts on this?

[shredder2500]

popping in again to see if there is any progress. hope yall dont mind. I just need this in order to be able to use this.

[mrschofield]

Hi guys,

I'm also just popping in to see where this is at? Is there anything i can do?

[ctaylor28]

any updates?

[jamesmbourne]

Sorry for the lack of updates on this - I'm currently occupied with other projects and commitments.

The individually flag makes this significantly more complex that I otherwise anticipated and may need some additional input to figure out the best approach.

My suggestion would to generate some sort of 'manifest' file after the build phase which includes the necessary information from Webpack but it would be good to hear any other suggestions.

I'll endeavour to pick this back up as it does seem a feature that plenty of people would find useful.

[ctaylor28]

is this just waiting on the code coverage check now?

[ctaylor28]

is this PR dead?

[jamesmbourne]

is this PR dead?

Sorry @ctaylor28, I've been too busy to be working on this at the moment. I've picked it up again and will try and get the PR ready for review.

I think it just needs some tests putting in for the save() method for compile stats to get the coverage up, and then a review.

[ctaylor28]

is this PR dead?

Sorry @ctaylor28, I've been too busy to be working on this at the moment. I've picked it up again and will try and get the PR ready for review.

I think it just needs some tests putting in for the save() method for compile stats to get the coverage up, and then a review.

looks like you also have two failing tests

[jamesmbourne]

@HyperBrain I think this should be good for review now if you're available to do so.

[ctaylor28]

@HyperBrain any chance we can get this reviewed soon?

[jamesmbourne]

After having a think about this, we could probably simplify this to avoid the need to serialise the Webpack stats entirely.

1. When sls webpack is run, an artifact ZIP is stored in .serverless. Modify this to store it in .webpack as the .serverless directory is deleted by the Serverless framework which is beyond our control.
2. When sls package or sls deploy is run, copy the artifact archives from the .webpack directory to .serverless and notify the framework of the location of these artifacts. If --no-build is specified, we will skip the generation of these artifacts and use the existing ones contained in .webpack.

I will open a new PR with that implementation and close this one.

This approach should be much simpler as we do not need to get involved in node_modules packaging etc. as this will all be in the pre-existing artifact.

[ctaylor28]

@jamesmbourne have you made another PR or started work on the other way yet?

[vicary]

For future lost souls, this is addressed in and superseded by #560.

[shredder2500]

@vicary Thank You!