Skip to content

fix: error in angular c3 template#7342

Merged
petebacondarwin merged 4 commits intocloudflare:mainfrom
neomax7:issues-7341
Dec 2, 2024
Merged

fix: error in angular c3 template#7342
petebacondarwin merged 4 commits intocloudflare:mainfrom
neomax7:issues-7341

Conversation

@neomax7
Copy link
Contributor

@neomax7 neomax7 commented Nov 25, 2024

AngularAppEngine class does not have .render method, instead it should be .handle method

Fixes #[insert GH or internal issue link(s)].
#7341
Describe your change...
AngularAppEngine class does not have .render method, instead it should be .handle method

changed method name from ".render" to ".handle" as the official doc below
https://angular.dev/api/ssr/AngularAppEngine


  • Tests
    • TODO (before merge)
    • Tests included
    • Tests not necessary because: only changing template for create-cloudflare with angular framework
  • E2E Tests CI Job required? (Use "e2e" label or ask maintainer to run separately)
    • I don't know
    • Required
    • Not required because: only changing template for create-cloudflare with angular framework
  • Public documentation
    • TODO (before merge)
    • Cloudflare docs PR(s):
    • Documentation not necessary because: only changing template for create-cloudflare with angular framework

@neomax7 neomax7 requested a review from a team November 25, 2024 20:35
@neomax7 neomax7 requested a review from a team as a code owner November 25, 2024 20:35
@changeset-bot
Copy link

changeset-bot bot commented Nov 25, 2024

🦋 Changeset detected

Latest commit: dd094e9

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
create-cloudflare Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions
Copy link
Contributor

github-actions bot commented Nov 27, 2024

A wrangler prerelease is available for testing. You can install this latest build in your project with:

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12116574395/npm-package-wrangler-7342

You can reference the automatically updated head of this PR with:

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/prs/7342/npm-package-wrangler-7342

Or you can use npx with this latest build directly:

npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12116574395/npm-package-wrangler-7342 dev path/to/script.js
Additional artifacts:
npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12116574395/npm-package-create-cloudflare-7342 --no-auto-update
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12116574395/npm-package-cloudflare-kv-asset-handler-7342
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12116574395/npm-package-miniflare-7342
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12116574395/npm-package-cloudflare-pages-shared-7342
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12116574395/npm-package-cloudflare-vitest-pool-workers-7342
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12116574395/npm-package-cloudflare-workers-editor-shared-7342
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12116574395/npm-package-cloudflare-workers-shared-7342
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12116574395/npm-package-cloudflare-workflows-shared-7342

Note that these links will no longer work once the GitHub Actions artifact expires.


wrangler@3.91.0 includes the following runtime dependencies:

Package Constraint Resolved
miniflare workspace:* 3.20241106.1
workerd 1.20241106.1 1.20241106.1
workerd --version 1.20241106.1 2024-11-06

Please ensure constraints are pinned, and miniflare/workerd minor versions match.

@emily-shen emily-shen changed the title fixed: related to issues-7341 fix: error in angular c3 template Nov 27, 2024
Copy link
Contributor

@emily-shen emily-shen 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 fixing this 🎉

@emily-shen
Copy link
Contributor

Ah can you add a changeset actually? should be a patch, you can run npx changeset to generate one. Thanks!

@petebacondarwin
Copy link
Contributor

I am concerned that this didn't fail in CI before this fix.
E.g. https://github.com/cloudflare/workers-sdk/actions/runs/12071631960/job/33663814631#step:5:196

@emily-shen
Copy link
Contributor

I am concerned that this didn't fail in CI before this fix. E.g. https://github.com/cloudflare/workers-sdk/actions/runs/12071631960/job/33663814631#step:5:196

@petebacondarwin was just looking into it! Seems like the generated package.json doesn't have a preview or deploy script, so c3 wasn't able to run tests (it still should've failed when there was no deploy though 🤔). The whole generated package.json for angular looks a bit wrong actually - was planning to open a separate PR to fix. Re this PR, I tested this one manually and it's fixed the referenced issue.

@neomax7
Copy link
Contributor Author

neomax7 commented Nov 28, 2024

Ah can you add a changeset actually? should be a patch, you can run npx changeset to generate one. Thanks!

added changeset patch

@petebacondarwin
Copy link
Contributor

Yes you are right. The problem is that there is no preview script. (We don't run deploy scripts in CI tests any more since they were quite flaky). I'll land this fix (which I also verified locally) and then add a preview script to ensure it is tested.

@petebacondarwin petebacondarwin self-assigned this Nov 29, 2024
@petebacondarwin
Copy link
Contributor

I updated the tests to run locally and they fail because Angular seems to hang. Does a new C3 template (after this PR) workf for you @neomax7? I haven't managed to debug what is causing the hang.

@neomax7
Copy link
Contributor Author

neomax7 commented Nov 30, 2024

I updated the tests to run locally and they fail because Angular seems to hang. Does a new C3 template (after this PR) workf for you @neomax7? I haven't managed to debug what is causing the hang.

Hi @petebacondarwin
image
Here is the screenshot I tried it on my laptop.
it seems working without any issue.
I tried it on mac-sequoia 15.1.1 with node v20.18.1 (lts/iron)

@neomax7
Copy link
Contributor Author

neomax7 commented Nov 30, 2024

it must be node version.
the highlighted line must be the issue.
this is same for the current released build of C3 tool.
no prebuilt binaries for node v22 (lts/jod)
image

neomax7 and others added 3 commits December 2, 2024 09:37
AngularAppEngine class does not have .render, instead it should be
.handle method
- add a preview script so that it gets included in CI test
- remove unnecessary --`experimnental-local` CLI arg
- add missing `xhr2` dependency
@petebacondarwin petebacondarwin merged commit 499e12d into cloudflare:main Dec 2, 2024
@workers-devprod workers-devprod added the contribution [Holopin] Recognizes an open-source contribution, big or small label Dec 2, 2024
@holopin-bot
Copy link

holopin-bot bot commented Dec 2, 2024

Congratulations @neomax7, the maintainer of this repository has issued you a holobyte! Here it is: https://holopin.io/holobyte/cm46vyaqy26800cjr6is1psmb

This badge can only be claimed by you, so make sure that your GitHub account is linked to your Holopin account. You can manage those preferences here: https://holopin.io/account.
Or if you're new to Holopin, you can simply sign up with GitHub, which will do the trick!

@workers-devprod workers-devprod mentioned this pull request Dec 2, 2024
emily-shen added a commit that referenced this pull request Dec 4, 2024
* add telemetry commands

* changeset

* fix and test dates

* update changeset

* add global/project status

* default true

* remove changeset

* update wrangler telemetry status

feat: add `wrangler metrics` as an alias for `wrangler telemetry` (#7284)

* add metrics alias

* tests

* use each to test alias

feat: send metrics for command start/complete/error (#7267)

* stop collecting userId in telemetry

Co-authored-by: emily-shen <emily-shen@users.noreply.github.com>

* implement telemetry collection

* infer errorType based on the constructor name

* implement common event properties

* log common event properties

Co-authored-by: Edmund Hung <me@edmund.dev>

* respect metric enabled/disabled

* remove dispatcher.identify

* include SPARROW_SOURCE_KEY in PR pre-release build

* fix tests

* ensure debug log covers the request failed message

* replace SPARROW_SOURCE_KEY regardless whethe env exists

---------

Co-authored-by: Edmund Hung <edmund@cloudflare.com>
Co-authored-by: emily-shen <emily-shen@users.noreply.github.com>
Co-authored-by: Edmund Hung <me@edmund.dev>

fix nested properties (#7300)

feat: add banner to indicate when telemetry is on (#7302)

* add banner

* abort if telemetry disable

* basic sendNewEvent tests

* banner tests

* settle promises before exiting

* remove unnecessary banner logic

* just check if version is different

feat: add some more properties to telemetry events (#7320)

* isCI and isNonInteractive

* add argsUsed and argsCombination

* don't include args if value is false

* redact arg values if string

* lint

* isNonInteractive -> isInteractive

cleanup defineCommand

test duration

log metrics request failure

add draft telemetry.md

add node and os versions

don't send wrangler metrics from c3 if disabled

don't send c3 metrics from wrangler init

add config type

add more comments to send-event

move types out of send-event.ts

add comment about applyBeforeValidation

normalize into camelcase

refactor telemetry command

update tests and some comments

normalise all args

pr feedback

update telemetry.md

use useragent to get package manager

make sendEvent/sendNewEvent sync

rename sendEvent and sendNewEvent

fix lock file

remove flushPromises

changeset

fail silently

feat(wrangler): make resources identifier optional if x-provision flag is enabled (#7377)

Fix wrangler module import under npm monorepos (#7130)

* Update import resolution for files and package exports

In an npm workspace environment, wrangler will now be able to
successfully resolve package exports.

Previously, wrangler would only be able to resolve modules in a relative
`node_modules` directory and not workspace root `node_modules`
directory.

* Use esbuild plugin

chore(wrangler): fix type errors with experimental flags (#7391)

refactor(wrangler): Explicitely pick node compat plugins for each mode (#7387)

* refactor: cleanup & simplify

* refactor(wrangler): Explicitely pick node compat plugins for each mode

* Update packages/wrangler/src/deployment-bundle/esbuild-plugins/hybrid-nodejs-compat.ts

Co-authored-by: Pete Bacon Darwin <pete@bacondarwin.com>

* fixup! renamed vars (review feedback)

* fixup! format

---------

Co-authored-by: Pete Bacon Darwin <pete@bacondarwin.com>

fix: error in angular c3 template (#7342)

* fixed: related to issues-7341
AngularAppEngine class does not have .render, instead it should be
.handle method

* added: changeset

* Tidy up changeset

* fix Angular template

- add a preview script so that it gets included in CI test
- remove unnecessary --`experimnental-local` CLI arg
- add missing `xhr2` dependency

---------

Co-authored-by: Peter Bacon Darwin <pbacondarwin@cloudflare.com>

[C3] Bump create-qwik from 1.10.0 to 1.11.0 in /packages/create-cloudflare/src/frameworks (#7359)

* [C3] Bump create-qwik in /packages/create-cloudflare/src/frameworks

Bumps [create-qwik](https://github.com/QwikDev/qwik/tree/HEAD/packages/create-qwik) from 1.10.0 to 1.11.0.
- [Release notes](https://github.com/QwikDev/qwik/releases)
- [Changelog](https://github.com/QwikDev/qwik/blob/create-qwik@1.11.0/packages/create-qwik/CHANGELOG.md)
- [Commits](https://github.com/QwikDev/qwik/commits/create-qwik@1.11.0/packages/create-qwik)

---
updated-dependencies:
- dependency-name: create-qwik
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

* chore: update dependencies of "create-cloudflare" package

The following dependency versions have been updated:

| Dependency  | From   | To     |
| ----------- | ------ | ------ |
| create-qwik | 1.10.0 | 1.11.0 |

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Wrangler automated PR updater <wrangler@cloudflare.com>

Remove await from user Worker fetch in router-worker (#7410)

fix: update Angular experimental Workers + Assets template (#7409)

* fix: update Angular experimental Workers + Assets template

* ci: enable tests on experimental Workers + Assets C3 templates

fix: update queues max_batch_timeout in miniflare (#7399)

* update timeout limit

* changeset

* test

* remove only

Co-authored-by: Carmen Popoviciu <cpopoviciu@cloudflare.com>

---------

Co-authored-by: Carmen Popoviciu <cpopoviciu@cloudflare.com>

refactor: move projectRoot computation to config validation (#7415)

move telemetry.md out of src

fixups

tiody up readRawConfig

Rename serve_directly to experimental_serve_directly (#7429)

chore(deps): bump the workerd-and-workers-types group across 1 directory with 2 updates (#7418)

Updates `workerd` from 1.20241106.1 to 1.20241202.2

Version Packages (#7358)

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

ci: don't watch for changes on the workers-shared test:ci job (#7420)

refactor: remove missed redundant computation of `projectRoot` (#7421)

* refactor: remove missed redundant computation of `projectRoot`

* test: do not watch test files in workflow fixture test jobs

feat(wrangler): add inherit bindings support (#7385)

* feat(wrangler): add inherit bindings support

* add test

* add changeset

* rename file to bindings

Relax type on `observability.enabled` (#7436)

fix: C3 experimental template for Solid now uses correct preset (#7343)

fix: allow the asset directory to be omitted in Wrangler config for commands that don't need it (#7426)

fix: C3 experimental template for Nuxt now uses correct preset (#7332)

* fix: C3 experimental template for Nuxt now uses correct preset

* test: remove quarantine from Nuxt experimental template

using the github browser merge is always a bad idea

fix e2e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contribution [Holopin] Recognizes an open-source contribution, big or small

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants