-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Social Previews: Remove i18n-calypso dependency by removing search preview header #44608
Social Previews: Remove i18n-calypso dependency by removing search preview header #44608
Conversation
Here is how your PR affects size of JS and CSS bundles shipped to the user's browser: Async-loaded Components (~25 bytes removed 📉 [gzipped])
React components that are loaded lazily, when a certain part of UI is displayed for the first time. Legend What is parsed and gzip size?Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory. Generated by performance advisor bot at iscalypsofastyet.com. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for tackling this and taking on the NPM version bump.
I tried testing this by checking out the PR and booting Calypso. I then went to the "Preview" modal in a post and looked at the Google Search
result. Unfortunately I saw this even after refreshing and trying again a few times.
I think this is because these fixes are made in the package not in the Calypso components. That is correct because until this PR is merged then we are still using the old components in Calypso.
To avoid this problem and make this PR easier to test could we first ship the PR to get WP.com Calypso to use the NPM package components? Then we can easily test this PR.
Yes, these changes are intentionally only applied to the social-previews package as other products incl. Calypso will consume this package going forward. And yes, I don't mind shipping #44604 first. :) |
730ff28
to
177ff1e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Mark CSS and SCSS files as `sideEffects` to ensure they are not discarded during build processes tree-shaking. | ||
|
||
|
||
#### v1.0.0 (2019-07-22) | ||
#### v1.0.0 (2020-07-22) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for spotting this!
Changes proposed in this Pull Request
Testing instructions
Check if all tests pass:
yarn run jest -c=test/packages/jest.config.js packages/social-previews/test/index.js
Fixes #44489