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

--maxage option not working? #45

Closed
mustafa0x opened this issue Oct 16, 2019 · 10 comments
Closed

--maxage option not working? #45

mustafa0x opened this issue Oct 16, 2019 · 10 comments
Labels

Comments

@mustafa0x
Copy link

I have the following in package.json:

"start:dev": "sirv public --single --dev --host 0 --port 8080 -m 0"

However, the response headers lack cache-control. Is this a bug or did I misunderstand the usage of -m/--maxage?

On a related note, wouldn't it make sense to always pass a no cache header in dev mode?

@lukeed
Copy link
Owner

lukeed commented Oct 16, 2019

Hey, thanks!

So I do a sorta similar thing for --dev mode: It skips that maxAge / Cache-Control option entirely, which would effectively operate the same as a private Cache-Control directive.

Relevant section: https://github.com/lukeed/sirv/blob/master/packages/sirv/index.js#L79-L98
Notice that the opts.maxAge check happens after dev mode has been executed.

This is also true on the current @next release.

@mustafa0x
Copy link
Author

Is there a way to explicit send Cache-control: no-store for dev mode (or even maxage=0)? Currently, I'm forced to use Disable cache in dev tools to make sure I always get a fresh response.

(Just now Dev tools was caching the sourcemaps of my bundles.)

@lukeed
Copy link
Owner

lukeed commented Oct 16, 2019

Not currently, through the CLI. If you were using sirv programmatically, you'd do that within a blanket setHeaders option.

What would you suggest? A new flag, or baking it into --dev? My hesitation is that some will still want to send CC headers while in dev mode, since "dev" just signifies that files are always sent from the filesystem & not an in-memory cache. And max-age=0 is not really the same as private/no-store.

@mustafa0x
Copy link
Author

I feel that a no-store CC header should be default with --dev. As we all know, when we're developing we always want a fresh response. This is especially important where you can't force a refresh of the changes, e.g. when testing the app on the phone. This happened to me, and it was a bit frustrating. I was forced to add a cache-busting query string.

There do exist a few rare exceptions however, so perhaps that default can be overridden by passing maxage in or another option.

@lukeed
Copy link
Owner

lukeed commented Oct 16, 2019

Sounds good, will do 👍 Should it be private, too?

@mustafa0x
Copy link
Author

Thanks!

As in Cache-Control: private? I think Cache-Control: no-store suffices.

public

  • Indicates that the response may be cached by any cache, even if the response would normally be non-cacheable (e.g. if the response does not contain a max-age directive or the Expires header).

private

  • Indicates that the response is intended for a single user and must not be stored by a shared cache. A private cache may store the response.

no-cache

  • Forces caches to submit the request to the origin server for validation before releasing a cached copy.

no-store

  • The cache should not store anything about the client request or server response.

Source: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Cache-Control

@lukeed
Copy link
Owner

lukeed commented Oct 16, 2019

Great, agreed

Do you mind if I put this into next only? That's close to being released so I'd prefer to have less commit-friction between the two branches

@mustafa0x
Copy link
Author

mustafa0x commented Oct 16, 2019 via email

@lukeed lukeed added the has fix label Oct 16, 2019
lukeed added a commit that referenced this issue Oct 16, 2019
@lukeed
Copy link
Owner

lukeed commented Oct 16, 2019

Available on the new next version for both packages:

  • sirv-cli@1.0.0-next.1
  • sirv@1.0.0-next.1
npm install sirv-cli@next

Thanks!

@mustafa0x
Copy link
Author

🎉

lukeed added a commit that referenced this issue Jun 10, 2020
* chore(sirv): bump “@polka/url” for better decoding

* feat(sirv): inline `opts.single` behavior

* fix(sirv): unknown Content-Type as empty string

* feat(sirv-cli): run HTTP/2 server behind flags

* chore(sirv-cli): update readme

* feat: run `sade` in single-command mode

* feat(sirv): allow `--single` to accept path;

- Closes #35

* fix(sirv): respect existing `Content-Type` header;

- Closes #38
- Closes #39

* fix(sirv-cli): use "-K" for `--http2` alias

* feat: allow precompiled gzip & brotli files;

- Closes #3

* refactor(sirv): consolidate "dev & prod" modes;

- still optimized
- adds ETag support to "dev" mode
- adds brotli/gzip support to "dev" mode
- Related: #3

* chore: bump devdeps

* v1.0.0-next.0

* chore(docs): fix Polka demo usage;

- Closes #42

* fix(sirv): set "no-store" during `dev` mode;

- Closes #45

* v1.0.0-next.1

* fix(sirv): extract `list` into package

* fix(sirv): ignore deeply-nested dotfiles too

* chore(sirv): update `polka/url` dep;

lukeed/polka#119

* v1.0.0-next.2

* feat(sirv-cli): add `--pass` flag for passphrase

* v1.0.0-next.3

* fix(sirv): compare `ETag` against `If-None-Match` header (#56)

* fix(sirv-cli): use `host` for port availability

* Add support for ETag-based caching

When providing an ETag header on responses, the client automatically sends that header value on subsequent requests for the matching resource as an `If-None-Match` request header. If the ETag value received in `If-None-Match` is the same as the ETag computed by the disk crawl, a 304 "Not Modified" status can be returned along with an empty body to indicate that the requestor's cached value should be used.

* chore: use existing `isEtag` var

Co-authored-by: Luke Edwards <luke.edwards05@gmail.com>

* chore(sirv): squash declaration

* v1.0.0-next.4

* break: require Node 10.x minimum;

- Node 8.x is already End of Life (2019-12-31)

* v1.0.0-next.5

* feat(sirv): add type definitions;

- Closes #61

* chore: add `notice` to base readme re: website;

- Closes #62

* feat(sirv): add `filter` option to determine file list;

- supplants default `dotfiles` matcher, responsible for own logic
- Closes #50

* fix(sirv): replace `filter` w/ built-in ".well-known" allowance;

- Closes #50

* chore: rename files

* chore(sirv-cli): rename `boot` file

* chore(sirv): produce `build` files w/ "module" entry

* chore: run tests with esm loader

* chore: re-extract `middleware` helper file

* chore: begin integration test suites;

- include fixtures directory

* chore: use `pnpm` again for lerna;

This reverts commit a56872b.

# Conflicts:
#	lerna.json

* chore(action): install `pnpm` for ci

* chore: add `single` tests

* chore(action): use `curl` installer for pnpm

* chore: add "extensions" tests

* chore: add `dotfiles` test suite

* chore: add `dev` test suite

* chore: add `etag` test suite

* chore: add `brotli` and `gzip` suites

* chore: add `maxAge` and `immutable` test suites

* chore: add `ranges` test suite

* fix(test): remove `only` on dotfiles suite

* chore: import `setHeaders` and `onNoMatch` suites

* chore: cleanup test helpers

* chore: include "build" step in CI

* chore: initial CLI tests

* revert: allow normal resolution

* chore: try `actions/cache` recipe

* chore: add loose CLI tests

* fix(test): terminate child procs correctly

* chore: boot `http2` server within test

* fix(sirv): normalize encoded/accented characters for cache

* debug: server spawner really annoying

* feat: add "assets" option (#53)

* Add option to specify asset paths for SPAs

Allows asset path prefixes to be specified by the CLI when running in
single-page mode.

If a path is requested that doesn't exist, and that path matches one of
the asset path prefixes, the request will 404 instead of retuning a 200
containing the root index.

Fixes #44

* fix: ensure `assets` array & preload prep work

* chore: add `assets` tests

* chore: update options definition

* update `--single` & `--assets` text

Co-authored-by: Luke Edwards <luke.edwards05@gmail.com>

* refactor: rename "assets" to "ignores" option

* fix: include dotfiles & well-known in default ignores

* fix: move `ignores` check into `toAssume` builder

* refactor: hoist bound `lookup` function

* fix(sirv): do not fill cache when `dev` true

* v1.0.0-next.6

* fix(sirv): extension regexp

* v1.0.0-next.7

* fix(sirv): remove premature ignore check;

- ran before extensions could be applied
- effectively reverts to #53

* v1.0.0-next.8

* fix(sirv): allow definition to export type members

* v1.0.0-next.9

* fix(sirv): dedupe extensions list w/ brotli + gzip

* chore: update documentation

* chore: update bench figures;

- include dummy apps directly

* chore: fix spacing

Co-authored-by: Jason Miller <developit@users.noreply.github.com>
Co-authored-by: Carey Metcalfe <carey@cmetcalfe.ca>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants