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

feat(node): add deleteOutputPath option to @nrwl/node:webpack executor #10073

Merged
merged 2 commits into from
Jun 8, 2022

Conversation

jaytavares
Copy link
Contributor

ISSUES CLOSED: #9167

Current Behavior

Node webpack previous build output is left in place for subsequent builds.

Expected Behavior

Ability to clear build output prior to subsequent builds by setting deleteOutputPath to true.

Related Issue(s)

Fixes #9167

@vercel
Copy link

vercel bot commented Apr 29, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Updated
nx-dev ⬜️ Ignored (Inspect) Jun 8, 2022 at 7:02PM (UTC)

Copy link
Member

@AgentEnder AgentEnder left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution @jaytavares! 🎉

This looks good for the most part, I left a comment to assist in fixing the e2e errors. Once those are fixed we can get this merged.

Let me know if you need any help.

packages/node/src/executors/webpack/webpack.impl.ts Outdated Show resolved Hide resolved
@jaytavares
Copy link
Contributor Author

jaytavares commented Jun 8, 2022

Hey @AgentEnder, I made that fix you suggested and got everything passing; I went to rebase and discovered a ton of merge conflicts, then I saw this: #10229

It would seem things went a different direction favoring the use of the CleanWebpackPlugin to ensure the output directory is cleared ahead of time instead of using a deleteOutputPath option.

Thoughts?

@AgentEnder
Copy link
Member

Hey @jaytavares! The PR you linked did indeed introduce a few conflicts, but it just uses CleanWebpackPlugin as an example (w/o actually introducing it functionally). The method you use here is still in line with our other executors, so moving forward with this would be good.

@jaytavares
Copy link
Contributor Author

jaytavares commented Jun 8, 2022

Hey @jaytavares! The PR you linked did indeed introduce a few conflicts, but it just uses CleanWebpackPlugin as an example (w/o actually introducing it functionally). The method you use here is still in line with our other executors, so moving forward with this would be good.

Okay cool. I agree that the consistency is desirable. On that note, a deleteOutputPath option should probably be added to the @nrwl/js:tsc executor as well. @AgentEnder, Do you think we should expand this PR to include that or should that be handled separately?

    Ensure that generated package.json is retained
    Remove lib from e2e test since it uses @nrwl/js:tsc
    executor
    Add deleteOutputPath to schema normalization
@jaytavares
Copy link
Contributor Author

okay, @AgentEnder, e2e tests are passing. 👍

@github-actions
Copy link

This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add deleteOutputPath option to @nrwl/node:webpack executor
2 participants