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(angular): support esbuild middleware functions #21048

Merged
merged 1 commit into from
Jan 22, 2024

Conversation

dmitry-stepanenko
Copy link
Contributor

Current Behavior

Angular has added an ability to provide both plugins and middleware functions. However nx now only leverages the exposed plugins API.

Expected Behavior

Should be able to configure middleware functions in the @nx/angular:dev-server executor.

Related Issue(s)

Fixes #

Copy link

vercel bot commented Jan 8, 2024

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

Name Status Preview Updated (UTC)
nx-dev ✅ Ready (Inspect) Visit Preview Jan 22, 2024 4:19pm

@dmitry-stepanenko
Copy link
Contributor Author

Also, should I update the docs for dev-server with a mention of esbuildMiddleware prop similarly to https://nx.dev/nx-api/angular/executors/browser-esbuild ?

@dmitry-stepanenko
Copy link
Contributor Author

Hi @leosvelperez, any chance this will be merged for the 17.3.0 release?

@leosvelperez
Copy link
Member

Hi @leosvelperez, any chance this will be merged for the 17.3.0 release?

Yeah. I have it on my list for early next week. Thanks for contributing this!

@leosvelperez
Copy link
Member

hey @dmitry-stepanenko! Thanks again for contributing this!

Also, should I update the docs for dev-server with a mention of esbuildMiddleware prop similarly to https://nx.dev/nx-api/angular/executors/browser-esbuild ?

Yeah, the current dev-server description actually requires an overhaul. It's solely focused on the prior state where only webpack was supported. You can take a stab at that if you want, but I think it requires more changes than what this PR is for. I also want to improve the examples to be more like the ones in https://github.com/nrwl/nx/pull/20556/files#diff-f7afde405397d901e0e6d3a714023e3b06448c5976af8b894db9aede4cfc2e1c.

We can skip this for now and do it separately. Let me know if you'd like to take a stab at it. Otherwise, I'll make those changes once this is merged.

@dmitry-stepanenko dmitry-stepanenko requested a review from a team as a code owner January 22, 2024 15:33
@dmitry-stepanenko
Copy link
Contributor Author

Thanks @leosvelperez. I will leave the docs for you in this case so that there's no back and forth on wording

Copy link
Member

@leosvelperez leosvelperez left a comment

Choose a reason for hiding this comment

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

Sorry. One thing I forgot, we need to validate at runtime that the option was not provided when the workspace is using an Angular version that doesn't support it. Similar to what we do in https://github.com/nrwl/nx/blob/master/packages/angular/src/builders/webpack-server/webpack-server.impl.ts#L99.

@dmitry-stepanenko
Copy link
Contributor Author

Oh, my bad, should have noticed that 🙈

Copied the error from the browser-esbuild executor

Copy link
Member

@leosvelperez leosvelperez left a comment

Choose a reason for hiding this comment

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

LGTM! 🎉

@leosvelperez leosvelperez enabled auto-merge (squash) January 22, 2024 16:45
@leosvelperez leosvelperez merged commit c1238aa into nrwl:master Jan 22, 2024
6 checks passed
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 Jan 28, 2024
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.

3 participants