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

@Sse() not compatible with compression() middleware #5762

Closed
bchoii opened this issue Nov 23, 2020 · 1 comment
Closed

@Sse() not compatible with compression() middleware #5762

bchoii opened this issue Nov 23, 2020 · 1 comment
Labels
needs triage This issue has not been looked into

Comments

@bchoii
Copy link

bchoii commented Nov 23, 2020

Bug Report

Current behavior

Current SSE not compatible with compression() middleware (https://github.com/expressjs/compression).

If compression middleware is present, SSE events do not seem to reach the browser.

Expected behavior

Expecting SSE to work with compression() middleware

Possible Solutions

Option 1:
Filter sse from compression()

  app.use(
    compression({
      filter: (req, res) =>
        !res.getHeaders()['content-type']?.includes('text/event-stream'),
    }),
  );

Option 2:
Add "no-transform" to "Cache-Control" headers for SSE responses. Compression middleware will detect this header and skip compression on this particular response. See expressjs/compression#51

 response.set({
    'Cache-Control': 'no-transform',
  });
@bchoii bchoii added the needs triage This issue has not been looked into label Nov 23, 2020
@bchoii bchoii mentioned this issue Nov 23, 2020
3 tasks
@bchoii bchoii changed the title compression interferes with SSE compression() middleware interferes with SSE Nov 24, 2020
@bchoii bchoii changed the title compression() middleware interferes with SSE compression() middleware not compatible with SSE Nov 24, 2020
@bchoii bchoii changed the title compression() middleware not compatible with SSE @Sse() not compatible with compression() middleware Nov 25, 2020
@kamilmysliwiec
Copy link
Member

This will be fixed in the next patch/minor release 41d46c0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs triage This issue has not been looked into
Projects
None yet
Development

No branches or pull requests

2 participants