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

Remove Babel polyfill from Slideshow block #31690

Merged
merged 1 commit into from
Mar 22, 2019

Conversation

simison
Copy link
Member

@simison simison commented Mar 22, 2019

Partly resolves #31689

Initially, I thought we could remove this dependency when we move blocks to Jetpack, but @jsnajdr is upgrading Babel setup in Calypso and this method of polyfilling is being deprecated.

Changes proposed in this Pull Request

Testing instructions

  • Build blocks:
     npm ci
     npx lerna bootstrap --concurrency=2 --scope='@automattic/jetpack-blocks'
     npx lerna run build --stream --scope='@automattic/jetpack-blocks' -- -- --output-path /path/to/jetpack/_inc/blocks/ --watch
    
  • Apply Add wp-polyfill as a dependency for Slideshow block jetpack#11657 in Jetpack
  • Add Slideshow block in the editor (don't add any other blocks to mess up results)
  • Open the page with the block
  • Note from networks tab how wp-polyfill loads
  • Block works in IE11

@simison simison added [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Goal] Gutenberg Working towards full integration with Gutenberg Slideshow labels Mar 22, 2019
@simison simison added this to the Jetpack: 7.2 milestone Mar 22, 2019
@matticbot
Copy link
Contributor

@simison simison requested review from a team and jsnajdr March 22, 2019 16:53
@simison simison added [Pri] Normal Schedule for the next available opportuinity. [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Mar 22, 2019
Copy link
Contributor

@ockham ockham left a comment

Choose a reason for hiding this comment

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

Works fine with Automattic/jetpack#11657 👍

@simison simison added [Status] Ready to Merge and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Mar 22, 2019
@simison simison merged commit 4280e0a into master Mar 22, 2019
@simison simison deleted the update/slideshow-remove-babel-polyfill branch March 22, 2019 19:37
jsnajdr added a commit that referenced this pull request Mar 25, 2019
The only usage got removed in #31690. Not needed any more in Jetpack Blocks.
@jsnajdr
Copy link
Member

jsnajdr commented Mar 25, 2019

👋 @simison and @ockham ! After the @babel/polyfill usage got removed here, it doesn't need to be as a dependency in package.json any more. Done in a follow-up PR #31711. Please 👀

jsnajdr added a commit that referenced this pull request Mar 25, 2019
…31711)

The only usage got removed in #31690. Not needed any more in Jetpack Blocks.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Goal] Gutenberg Working towards full integration with Gutenberg [Pri] Normal Schedule for the next available opportuinity. Slideshow [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Swap babel/polyfill to wp-polyfill for Slideshow block
4 participants