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

fix: warn on custom cache, add option for possibleTypes #2145

Open
wants to merge 1 commit into
base: release-bot/next-v15.x
Choose a base branch
from

Conversation

robertkowalski
Copy link
Contributor

@robertkowalski robertkowalski commented May 12, 2022

when using a custom cache in the render, memory would grow very
fast.

run:

yarn
rm -rf node_modules/.cache/ && yarn build -p
clinic doctor -- node ./node_modules/.bin/hops serve

and in a seperate window a few times:

autocannon -c 50 http://localhost:8080/jobs/test

current users are now warned when they pass in a custom cache
instance and we are offering an option to pass in possibleTypes
via options.

in hops 16, we can remove custom caches.

Co-authored-by: Jonas Holland jonas.holland@new-work.se

This pull request closes issue # (put the issue number here)

Current state

Changes introduced here

Checklist

  • All commit messages adhere to the appropriate format in order to trigger a correct release
  • All code is written in untranspiled ECMAScript (ES 2017) and is formatted using prettier
  • Necessary unit tests are added in order to ensure correct behavior
  • Documentation has been added
Bors merge bot cheat sheet

We are using bors-ng to automate merging of our pull requests. The following table provides a summary of commands that are available to reviewers (members of this repository with push access) and delegates (in case of bors delegate+ or bors delegate=[list]).

Syntax Description
bors merge Run the test suite and push to master if it passes. Short for "reviewed: looks good."
bors merge- Cancel an r+, r=, merge, or merge=
bors try Run the test suite without pushing to master.
bors try- Cancel a try
bors delegate+ Allow the pull request author to merge their changes.
bors delegate=[list] Allow the listed users to r+ this pull request's changes.
bors retry Run the previous command a second time.

This is a short collection of opinionated commands. For a full list of the commands read the bors reference.

cache ??
new InMemoryCache({
possibleTypes: this.options.possibleTypes || possibleTypes,
})
Copy link
Contributor Author

Choose a reason for hiding this comment

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

maybe we want

      new InMemoryCache(
        this.options.cacheOptions || { possibleTypes },
      )

here which gives the user more flexibility about cache options and might be more future proof

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ZauberNerd @jhiode what do you think?

when using a custom cache in the render, memory would grow very
fast.

to reproduce, clone GHE/marc-molins/hops-15/

run:

```
yarn
rm -rf node_modules/.cache/ && yarn build -p
clinic doctor -- node ./node_modules/.bin/hops serve
```

and in a seperate window a few times:

```
autocannon -c 50 http://localhost:8080/jobs/test
```

---

current users are now warned when they pass in a custom cache
instance and we are offering an option to pass in `possibleTypes`
via options.

in hops 16, we can remove custom caches.

Co-authored-by: Jonas Holland <jonas.holland@new-work.se>
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.

None yet

1 participant