Masterbar: Copy module code to package

[fgiannar]

This PR copies the Masterbar module codebase to the masterbar package. In the process:

• Methods that only live in the Jetpack plugin have been replaced by corresponding methods available in packages. For example: Jetpack::is_module_active has been replaced by ( new Modules() )->is_module_active
• The JETPACK__VERSION used for cache busting when enqueueing scripts and styles is now replaced by Main::PACKAGE_VERSION
• Translations text domain is updated to jetpack-masterbar from jetpack
• Updated the build process so that rtl files are now generated with a custom config (we used to refer to them as weird RTL entries because -rtl was appended instead of .rtl) and the corresponding code references as well.
• Updated all scripts and css files to be loaded from the dist folder
• Started enqueuing all package scripts and styles via the Assets package
• Fixed close to 270 phan issues
• Added more unit tests, related mostly to enqueuing assets

Other information:

• Have you written new tests for your changes, if applicable?
• Have you checked the E2E test CI results, and verified that your changes do not break them?
• Have you tested your changes on WordPress.com, if applicable (if so, you'll see a generated comment below with a script to run)?

Jetpack product discussion

pfwV0U-3U-p2

Does this pull request change what data or activity we track or use?

No

Testing instructions:

• Code Review since the package is not used anywhere yet.
• Migrated unit tests should all pass
• Try running jetpack build packages/masterbar with/without --production and inspect the generated assets in the dist folder

[github-actions]

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

• ✅ Include a description of your PR changes.
  
• ✅ Add a "[Status]" label (In Progress, Needs Team Review, ...).
  
• ✅ Add testing instructions.
  
• ✅ Specify whether this PR includes any changes to data or privacy.
  
• ✅ Add changelog entries to affected projects
  

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖

---

The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available.

---

Once your PR is ready for review, check one last time that all required checks appearing at the bottom of this PR are passing or skipped.
Then, add the "[Status] Needs Team Review" label and ask someone from your team review the code. Once reviewed, it can then be merged.
If you need an extra review from someone familiar with the codebase, you can update the labels from "[Status] Needs Team Review" to "[Status] Needs Review", and in that case Jetpack Approvers will do a final review of your PR.

[github-actions]

Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.

• To test on WoA, go to the Plugins menu on a WordPress.com Simple site. Click on the "Upload" button and follow the upgrade flow to be able to upload, install, and activate the Jetpack Beta plugin. Once the plugin is active, go to Jetpack > Jetpack Beta, select your plugin, and enable the update/copy-masterbar-code-to-pkg branch.

  • For jetpack-mu-wpcom changes, also add define( 'JETPACK_MU_WPCOM_LOAD_VIA_BETA_PLUGIN', true ); to your wp-config.php file.

• To test on Simple, run the following command on your sandbox:

  bin/jetpack-downloader test jetpack update/copy-masterbar-code-to-pkg
  

  bin/jetpack-downloader test jetpack-mu-wpcom-plugin update/copy-masterbar-code-to-pkg
  

Interested in more tips and information?

• In your local development environment, use the jetpack rsync command to sync your changes to a WoA dev blog.
• Read more about our development workflow here: PCYsg-eg0-p2
• Figure out when your changes will be shipped to customers here: PCYsg-eg5-p2

[fgiannar]

@Automattic/jetpack-vulcan How do we usually handle the ESLint failing tests related to missing JSDoc elements? Should I manually fix those or add an ignore rule?

[fgiannar]

Add eslint-disable rules related to JSDoc requires

@Automattic/jetpack-vulcan How do we usually handle the ESLint failing tests related to missing JSDoc elements? Should I manually fix those or add an ignore rule?

FYI I added eslint-disable rules via cae099f and 513f7a5

[coder-karen]

I've not looked through the full list of phan flagged issues, but certainly some look like false positives, eg Error: UndefError PhanUndeclaredClassConstant Reference to constant MANAGE_PLUGINS from undeclared class \WPCOM_Features (as it's a check to see if it exists). If nothing else stands out as being fixable, if you add a baseline.php file then run jetpack phan --update-baseline packages/masterbar it will create the phan baseline file and add all the relevant suppressions.

[fgiannar]

@darssen Many thanks for the review! Really appreciate the time you took!

left some comments, mainly regarding missing require_once and some inconsistencies in the @Package annotation. I only pinpointed two of the different @Package annotations that need to be looked at to avoid making too much noise in the PR but it would be nice to make sure all of them are properly set.

I believe these are all addressed now. I've also left responses to all your inline comments!

Also, when reviewing differences between the module and the package I found two fixes that have been done in the module that we would like to port to the package code. (#37314, #37315).

Amazing thanks! Done with a2b5a0f

Would you mind giving this a final look?

[darssen]

Thanks for all the changes. Code looks good 👍

We can 🚢 this one, since most of the testing will be done once we actually start using the package.

We need to remember to keep an eye on future changes in the module before we start using the package.

[fgiannar]

@anomiex Since you were also involved in this PR would you mind giving it a final check too? Many thanks!

[anomiex]

LGTM. I left some suggestions inline for further improvement, but I'd be fine with merging this and iterating.

Note I mainly looked at the structure, I didn't look through the code.

[anomiex]

Some additional stuff you might production-exclude here:

• tools/**
• Probably also src/**/*.scss and src/**/*.css and src/**/*.js, since they're all compiled into JS in dist/ now.

[fgiannar]

Done via 8265d6c

[fgiannar]

LGTM. I left some suggestions inline for further improvement, but I'd be fine with merging this and iterating.

Thanks @anomiex ! I wanted to update the README anyways and thought it would be better to implement all suggestions before merging :)

[anomiex]

LGTM, haven't tested though.

All the test failures are unrelated. Chances are a trunk merge after #37659 is merged will be needed for those.