Add automatic dependency management/externals generation to build process 🎉(fixes #1052) #1286

nerrad · 2019-06-04T15:44:58Z

Problem this Pull Request solves

See #1052 for background. This pull will make php dependency management for scripts and styles, and js webpack configuration for externals much easier (and less prone to human error).

Two primary benefits introduced in this pull:

Dependencies for scripts and styles are automatically registered php side.

With a few exceptions, you'll never have to touch php again for any current registered scripts as it's all handled automatically. The exceptions are:

Any script dependent on eventespresso-data-stores. Since this is not imported in implementations, it doesn't get picked up by the dependency extraction. So there may be a few cases where it may need added as a dependency manually. However, in most cases (since it, itself is dependent on wp.data) things should still get enqueued in the correct order.
Any style dependent on eventespresso-core-default-theme will need to have that dependency added manually because it is not imported in any implementations (I have some ideas for improving this in future iterations, but its tricky so I'll leave out of this pull for now)

Externals no longer need to be explicitly defined in the webpack config 🎉

Externals are picked up automatically via the dependency extraction webpack plugin (and the configuration for it). If any new packages are added they'll need to be updated in the configuration (I'll take care of doing this for the EDTR branch), but externals are automatically generated correctly for built files using the new webpack plugin.

To make all this possible, there were a number of corollary changes made in php files and js configuration including:

new methods (php side) for getting the dependencies from the json files.
making the chunk name strings and asset handle labels consistent (reduces the complexity of mapping).
some work on making the webpack configs more dry.
I also removed redux and react-redux from builds (making the vendor file much smaller) because we don't currently use them anywhere.

How has this been tested

Builds should run fine
js unit tests should pass

This impacts all current released code using built files so:

Verify plugin exit modal works as expected
Verify the EventAttendees block works as expected.

Checklist

I have read the documentation relating to systems affected by this pull request, see https://github.com/eventespresso/event-espresso-core/tree/master/docs
User input is adequately validated and sanitized
all publicly displayed strings are internationalized (usually using esc_html__(), see https://codex.wordpress.org/I18n_for_WordPress_Developers)
My code is tested.
My code follows the Event Espresso code style.
My code has proper inline documentation.
My code accounts for when the site is in Maintenance Mode (MM2 especially disallows usage of models)

Neither of these packages are used so we can stop including them everywhere. They can be added back in if we ever have need for them.

…script handle map.

tn3rb

AWESOME STUFF

LOVE IT!!!

one suggestion I have to make things a bit more DRY would be to replace all of the repeated blocks of code in EventEspresso\core\domain\services\assets\CoreAssetManager where the only thing changing is the asset handle.

For example, if we added a new method to EventEspresso\core\services\assets\AssetManager that was maybe something like:

public function addStandardJavascript(
        $handle,
        array $extra_dependencies = array(),
        $load_in_footer = true
    ) {
        $dependencies = ! empty($extra_dependencies) && is_array($extra_dependencies)
            ? array_merge(
                $this->registry->getJsDependencies(
                    $this->domain->assetNamespace(),
                    $handle
                ),
                $extra_dependencies
            )
            : $this->registry->getJsDependencies(
                $this->domain->assetNamespace(),
                $handle
            );
        return $this->addJavascript(
            $handle,
            $this->registry->getJsUrl($this->domain->assetNamespace(), $handle),
            $dependencies,
            $load_in_footer
        );
    }

then the following code in CoreAssetManager::loadCoreJs():

$this->addJavascript(
    CoreAssetManager::JS_HANDLE_JS_CORE,
    $this->registry->getJsUrl($this->domain->assetNamespace(), CoreAssetManager::JS_HANDLE_JS_CORE),
    $this->registry->getJsDependencies(
        $this->domain->assetNamespace(),
        CoreAssetManager::JS_HANDLE_JS_CORE
    )
)
->setHasInlineData();

$this->addJavascript(
    CoreAssetManager::JS_HANDLE_VENDOR,
    $this->registry->getJsUrl($this->domain->assetNamespace(), CoreAssetManager::JS_HANDLE_VENDOR),
    $this->registry->getJsDependencies(
        $this->domain->assetNamespace(),
        CoreAssetManager::JS_HANDLE_VENDOR
    )
);

$this->addJavascript(
    CoreAssetManager::JS_HANDLE_VALIDATORS,
    $this->registry->getJsUrl($this->domain->assetNamespace(), CoreAssetManager::JS_HANDLE_VALIDATORS),
    $this->registry->getJsDependencies(
        $this->domain->assetNamespace(),
        CoreAssetManager::JS_HANDLE_VALIDATORS
    )
)->setRequiresTranslation();

$this->addJavascript(
    CoreAssetManager::JS_HANDLE_HELPERS,
    $this->registry->getJsUrl($this->domain->assetNamespace(), CoreAssetManager::JS_HANDLE_HELPERS),
    $this->registry->getJsDependencies(
        $this->domain->assetNamespace(),
        CoreAssetManager::JS_HANDLE_HELPERS
    )
)->setRequiresTranslation();

could be changed to something like:

$this->addStandardJavascript(CoreAssetManager::JS_HANDLE_JS_CORE)->setHasInlineData();
$this->addStandardJavascript(CoreAssetManager::JS_HANDLE_VENDOR);
$this->addStandardJavascript(CoreAssetManager::JS_HANDLE_VALIDATORS)->setRequiresTranslation();
$this->addStandardJavascript(CoreAssetManager::JS_HANDLE_HELPERS)->setRequiresTranslation();

since the only thing changing in those blocks of code is the asset handle, and what is done to the asset after it is registered.

I couldn't come up with a better name for addStandardJavascript() (addAutoJavascript(), addJavascriptByHandle()???) but you might have a better idea.

Something similar could be done with the CSS assets

core/domain/services/assets/CoreAssetManager.php

core/services/assets/Registry.php

webpack.prod.js

…est file

core/domain/services/assets/CoreAssetManager.php

core/services/assets/BlockAssetManager.php

tn3rb · 2019-06-04T18:20:52Z

core/services/assets/Registry.php

+     * This is a known array of possible wp css handles that correspond to what may be exposed as dependencies in our
+     * build process.  Currently the dependency export process in webpack does not consider css imports, so we derive
+     * them via the js dependencies (WP uses the same handle for both js and css). This is a list of known handles that
+     * are used for both js and css.


good description 👍

core/services/assets/Registry.php

tn3rb

looks great!!!

nerrad · 2019-06-04T19:59:28Z

I gave this a priority of high because the longer this lingers the greater the chance of conflicts. Also, I'd like to merge this into the EDTR refactor work being done (which will already be a tricky merge).

lorenzocaum · 2019-06-04T20:11:11Z

@nerrad

I tested the Event Attendees block and that worked:

I also tested the exit modal with the skip option and then the yes option:

…atic dependency management/externals generation to build process 🎉(fixes #1052) (#1286) * various changes/updates to webpack configs * Add dependency extraction webpack plugin * remove unused dependencies for react-redux and redux Neither of these packages are used so we can stop including them everywhere. They can be added back in if we ever have need for them. * dependency map extractor configuration for externals and external to script handle map. * update all depedency registrations * update build files (dev) * add back in changes removed in a bad rebase * make webpack config more dry. * update build files (dev) * remove unneccesary delete * fix typo (copy pasta) in webpack.prod.js * some more DRY work for asset manager * relocate wp css handles for dependency generation * add default return for condition where there isn’t a registered manifest file * remove unnecessary Domain injection * more implementations of asset helpers "

sirreal

Very cool to see this, hope it's working well 😁

sirreal · 2019-06-05T09:36:14Z

bin/asset-dependency-maps.js

@@ -0,0 +1,44 @@
+const requestToExternal = ( request ) => {


nerrad · 2019-06-05T11:07:15Z

Very cool to see this, hope it's working well 😁

Hey! It's working very well! Thanks for creating the plugin.

…cidental change of method scope (#1286) "

nerrad added 9 commits June 4, 2019 09:47

various changes/updates to webpack configs

08981d4

Add dependency extraction webpack plugin

e5dd58d

remove unused dependencies for react-redux and redux

e260d31

Neither of these packages are used so we can stop including them everywhere. They can be added back in if we ever have need for them.

dependency map extractor configuration for externals and external to …

fc6167d

…script handle map.

update all depedency registrations

dab0801

update build files (dev)

ea5aaf4

add back in changes removed in a bad rebase

8bae74e

make webpack config more dry.

3ed6f04

update build files (dev)

85f2159

nerrad added type:enhancement 🎉 type:build-process 🔨 category:assets labels Jun 4, 2019

nerrad added this to the 4.9.82.p milestone Jun 4, 2019

nerrad requested a review from tn3rb June 4, 2019 15:44

nerrad assigned nerrad and tn3rb Jun 4, 2019

remove unneccesary delete

b0eb2dd

tn3rb requested changes Jun 4, 2019

View reviewed changes

core/domain/services/assets/CoreAssetManager.php Outdated Show resolved Hide resolved

core/services/assets/Registry.php Outdated Show resolved Hide resolved

webpack.prod.js Outdated Show resolved Hide resolved

tn3rb removed their assignment Jun 4, 2019

nerrad added 2 commits June 4, 2019 13:23

fix typo (copy pasta) in webpack.prod.js

5f35257

some more DRY work for asset manager

eae49f2

nerrad requested a review from tn3rb June 4, 2019 18:04

nerrad assigned tn3rb Jun 4, 2019

nerrad added 2 commits June 4, 2019 14:10

relocate wp css handles for dependency generation

34abde8

add default return for condition where there isn’t a registered manif…

fde34e6

…est file

tn3rb requested changes Jun 4, 2019

View reviewed changes

tn3rb removed their assignment Jun 4, 2019

nerrad added 2 commits June 4, 2019 15:48

remove unnecessary Domain injection

15b8d47

more implementations of asset helpers

3cc025c

nerrad requested a review from tn3rb June 4, 2019 19:52

nerrad assigned tn3rb Jun 4, 2019

tn3rb approved these changes Jun 4, 2019

View reviewed changes

tn3rb removed their assignment Jun 4, 2019

nerrad assigned joshfeck and unassigned nerrad Jun 4, 2019

nerrad added the priority:high label Jun 4, 2019

nerrad merged commit b2701ae into master Jun 4, 2019

nerrad deleted the FET/add-automatic-dependency-management branch June 4, 2019 20:20

sirreal reviewed Jun 5, 2019

View reviewed changes

bin/asset-dependency-maps.js

@@ -0,0 +1,44 @@

const requestToExternal = ( request ) => {

Copy link

sirreal Jun 5, 2019

Choose a reason for hiding this comment

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

😎

nerrad added a commit that referenced this pull request Jun 5, 2019

revert accidental change of method scope (#1286)

eee9f79

eeteamcodebase pushed a commit that referenced this pull request Jun 5, 2019

Bumping version to 4.9.82.rc.029. Previous Commit message: "revert ac…

9ac7b1e

…cidental change of method scope (#1286) "

joshfeck mentioned this pull request Jul 10, 2019

No attendee block styles on the front end? #1375

Closed

nerrad mentioned this pull request Jul 17, 2019

BUG: Fix block styles not loading on frontend (fixed #1375) #1422

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add automatic dependency management/externals generation to build process 🎉(fixes #1052) #1286

Add automatic dependency management/externals generation to build process 🎉(fixes #1052) #1286

nerrad commented Jun 4, 2019 •

edited

Loading

tn3rb left a comment •

edited

Loading

tn3rb Jun 4, 2019

tn3rb left a comment

nerrad commented Jun 4, 2019

lorenzocaum commented Jun 4, 2019

sirreal left a comment

sirreal Jun 5, 2019

nerrad commented Jun 5, 2019

Add automatic dependency management/externals generation to build process 🎉(fixes #1052) #1286

Add automatic dependency management/externals generation to build process 🎉(fixes #1052) #1286

Conversation

nerrad commented Jun 4, 2019 • edited Loading

Problem this Pull Request solves

Dependencies for scripts and styles are automatically registered php side.

Externals no longer need to be explicitly defined in the webpack config 🎉

How has this been tested

Checklist

tn3rb left a comment • edited Loading

Choose a reason for hiding this comment

AWESOME STUFF

tn3rb Jun 4, 2019

Choose a reason for hiding this comment

tn3rb left a comment

Choose a reason for hiding this comment

nerrad commented Jun 4, 2019

lorenzocaum commented Jun 4, 2019

sirreal left a comment

Choose a reason for hiding this comment

sirreal Jun 5, 2019

Choose a reason for hiding this comment

nerrad commented Jun 5, 2019

nerrad commented Jun 4, 2019 •

edited

Loading

tn3rb left a comment •

edited

Loading