Skip to content

fix: replacing compression with modern version#6557

Merged
patak-cat merged 2 commits intovitejs:mainfrom
Niputi:feat/compression-mordern-version
Mar 8, 2022
Merged

fix: replacing compression with modern version#6557
patak-cat merged 2 commits intovitejs:mainfrom
Niputi:feat/compression-mordern-version

Conversation

@Niputi
Copy link
Copy Markdown
Contributor

@Niputi Niputi commented Jan 18, 2022

Description

correctly fix: #2754

once lukeed/polka#148 have merged and released as a polka package, we can use their package instead.
code is copied and slightly modified from the pull request

Additional context


What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Read the Pull Request Guidelines and follow the Commit Convention.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Ideally, include relevant tests that fail without this PR but pass with it.

@Niputi Niputi changed the title replacing compression with modern version fix: replacing compression with modern version Jan 18, 2022
@Niputi
Copy link
Copy Markdown
Contributor Author

Niputi commented Jan 18, 2022

added compression to the root as I don't believe it's possible to import vite middlewares in playground ssr-vue & ssr-react

@TrySound
Copy link
Copy Markdown
Contributor

Btw can you check rm -rf dist/node && pnpm run build-bundle && du -ck dist/node on main and in your branch?

@Niputi Niputi requested a review from patak-cat January 26, 2022 13:20
@patak-cat
Copy link
Copy Markdown
Member

@Niputi in the PR developit mentions a fix in the inlined version of WMR of it that wasn't backported. Did you check what that was about? lukeed/polka#148 (comment)

@Niputi
Copy link
Copy Markdown
Contributor Author

Niputi commented Jan 26, 2022

I can see a few differences when comparing this to https://github.com/preactjs/wmr/pull/155/files
will update later to match with the wmr pull request

@patak-cat
Copy link
Copy Markdown
Member

@Niputi maybe you could directly copy the current version from WMR adding their license in it: https://github.com/preactjs/wmr/blob/main/packages/wmr/src/lib/polkompress.js

There are other changes it seems compared to the one in your last comment.

@Niputi Niputi force-pushed the feat/compression-mordern-version branch from 30cb9d6 to 82b3c99 Compare January 31, 2022 22:46
@Niputi Niputi added the p3-minor-bug An edge case that only affects very specific usage (priority) label Jan 31, 2022
@Niputi
Copy link
Copy Markdown
Contributor Author

Niputi commented Feb 2, 2022

@patak-dev is this change good?

@patak-cat
Copy link
Copy Markdown
Member

I think it is good @Niputi, thanks for updating it! Let's wait for the next minor for it so we more testing. I think we should stop merging new changes until we release 2.8. We should start 2.9 rather soon and avoid being one month again in beta

@patak-cat patak-cat added this to the 2.9 milestone Feb 2, 2022
@patak-cat
Copy link
Copy Markdown
Member

@Niputi would you resolve the conflicts so we merge this PR? Thanks!

@Niputi Niputi force-pushed the feat/compression-mordern-version branch from b85294d to 1e45e37 Compare March 5, 2022 21:39
@Niputi
Copy link
Copy Markdown
Contributor Author

Niputi commented Mar 5, 2022

done @patak-dev

@patak-cat patak-cat requested a review from bluwy March 7, 2022 21:30
@patak-cat patak-cat merged commit 5648d09 into vitejs:main Mar 8, 2022
@Niputi Niputi deleted the feat/compression-mordern-version branch March 8, 2022 07:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

p3-minor-bug An edge case that only affects very specific usage (priority)

Projects

No open projects
Status: Todo

Development

Successfully merging this pull request may close these issues.

this._implicitHeader is not a function

4 participants