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

Switch away from @eventespresso/i18n and use @wordpress/i18n directly (wip) #769

Conversation

nerrad
Copy link
Contributor

@nerrad nerrad commented Nov 2, 2018

Problem this Pull Request solves

A while ago, I started using @eventespresso/i18n as a way to get around the fact that we started using wp.i18n functionality in environments that may not have wp.i18n so it was a way to ensure we could just use it anywhere (basically eejs.i18n is just a wrapper around the @wordpress/i18n package).

However, now that the release of WP5.0 is imminent, I think it'd be better for long term maintenance if we start using @wordpress/i18n everywhere instead and I can just give it the same treatment that I did for bundled vendor scripts in WordPress (such as react). So essentially just copying the wp.i18n package into the assets/vendors directory and registering it on the wpi18n handle if its not already registered in WP.

Note: this is not an urgent requirement and can be done "whenever". It's more of an optimization thing (and reduces maintenance overhead). When I implement, I can just alias @eventespresso/i18n to the wp.i18n external instead of eejs.i18n.

How has this been tested

  • Verify no js unit-tests fail (indicating configuration problems)
  • Verify files build successfully
  • Verify existing GB blocks and react apps (plugins deactivation modal) still work as expected on the newly built files.

Checklist

@stale
Copy link

stale bot commented Jan 1, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@nerrad
Copy link
Contributor Author

nerrad commented Feb 7, 2019

I'm thinking it'd actually be good to put this ticket on the backburner until we bump minimum version required in EE to WP 5.0. What we have now works fine across all WP versions we support and it reduces the need for additional boilerplate code to handle when EE is run in an environment that doesn't have wp.i18n.

wp.i18n itself is not a big script so while moving to just it will save some size in the js, I'm not sure it's enough of a deal to use it currently.

@nerrad
Copy link
Contributor Author

nerrad commented Feb 7, 2019

@joshfeck based on my previous comment, what pipeline do you think this would be best in? I don't want to close it because it is worthwhile doing when its easy to do so (i.e. we bump minimum wp version required for EE).

@tn3rb
Copy link
Member

tn3rb commented Nov 30, 2020

no longer applicable

@tn3rb tn3rb closed this Nov 30, 2020
@tn3rb tn3rb deleted the Gutenberg/switch-eventespresso.i18n-to-wp.i18n branch July 8, 2021 19:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants