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

Add Interactivity API support #451

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

parthnvaswani
Copy link

@parthnvaswani parthnvaswani commented Apr 9, 2024

Description

This PR addresses the task of updating node packages and integrating support for JavaScript modules into the project. The focus is on enhancing theme scripts for the Interactivity API while ensuring compatibility with the latest dependencies, without breaking existing functionality.

Technical Details

  • Updated package.json to include the latest dependencies without affecting existing functionality.
  • Updated wp-script dependency to version 27.6.0.
  • Added @wordpress/interactivity as a dependency.
  • Modified scripts to include the --experimental-modules flag for compatibility with JavaScript modules.
  • Configured Webpack to enable experimental module features as follows:
    experiments: {
        outputModule: true,
    },
    output: {
        module: true,
    },
  • Added checks in Webpack configuration to support both module and non-module scripts.
  • Implemented proper enqueueing of scripts using wp_enqueue_script_module.
  • Organized changes within Theme Elementary, placing them in a subfolder named modules in assets/src/js.
  • Added an example script using the Interactivity API in the modules folder to demonstrate the implementation of JavaScript modules in the project.

Checklist

  • Update package.json to include the latest dependencies without breaking existing functionality.
  • Update wp-script dependency to version 27.6.0.
  • Add @wordpress/interactivity as a dependency.
  • Modify scripts to include the --experimental-modules flag.
  • Configure Webpack to enable experimental module features.
  • Add checks in Webpack configuration to support both module and non-module scripts.
  • Implement proper enqueueing of scripts using wp_enqueue_script_module.
  • Organize changes within Theme Elementary, placing them in a subfolder named modules in assets/src/js.
  • Add an example script using the Interactivity API in the modules folder.

Screenshots/Recordings

Screen.Recording.2024-04-09.at.5.57.50.PM.mov

Fixes/Covers issue

Fixes #450

@parthnvaswani parthnvaswani self-assigned this Apr 9, 2024
@SH4LIN
Copy link
Contributor

SH4LIN commented Jul 16, 2024

@parthnvaswani Can you look into why the PHP Unit test is failing? Also, add title of adding support for Interactivity API.

@thelovekesh thelovekesh marked this pull request as draft July 16, 2024 16:18
@thelovekesh thelovekesh marked this pull request as ready for review July 16, 2024 16:18
@SH4LIN
Copy link
Contributor

SH4LIN commented Jul 21, 2024

@rtCamp/team-sys We are facing an issue with the PHP Unit test, it is failing because of this issue https://github.com/rtCamp/theme-elementary/actions/runs/9960495534/job/27712384396#step:6:58

ERROR: for d3b0d4ac2f17bdfbd1266d369562057c_mysql_1  'ContainerConfig'
Creating d3b0d4ac2f17bdfbd1266d369[56](https://github.com/rtCamp/theme-elementary/actions/runs/9960495534/job/27712384396#step:6:57)2057c_tests-wordpress_1 ... done
[2628] Failed to execute script docker-compose

ERROR: for mysql  'ContainerConfig'
Traceback (most recent call last):
  File "docker-compose", line 3, in <module>
  File "compose/cli/main.py", line 81, in main
  File "compose/cli/main.py", line 203, in perform_command
  File "compose/metrics/decorator.py", line 18, in wrapper
  File "compose/cli/main.py", line 1186, in up
  File "compose/cli/main.py", line 1182, in up
  File "compose/project.py", line 702, in up
  File "compose/parallel.py", line 108, in parallel_execute
  File "compose/parallel.py", line 206, in producer
  File "compose/project.py", line 688, in do
  File "compose/service.py", line 581, in execute_convergence_plan
  File "compose/service.py", line 503, in _execute_convergence_recreate
  File "compose/parallel.py", line 108, in parallel_execute
  File "compose/parallel.py", line 206, in producer
  File "compose/service.py", line 496, in recreate
  File "compose/service.py", line 615, in recreate_container
  File "compose/service.py", line 334, in create_container
  File "compose/service.py", line 922, in _get_container_create_options
  File "compose/service.py", line 962, in _build_container_volume_options
  File "compose/service.py", line 1549, in merge_volume_bindings
  File "compose/service.py", line 1[57](https://github.com/rtCamp/theme-elementary/actions/runs/9960495534/job/27712384396#step:6:58)9, in get_container_data_volumes
KeyError: 'ContainerConfig'
Error: Process completed with exit code 255.
Screenshot 2024-07-21 at 2 37 54 PM

I am adding an error log here as well.

@thelovekesh thelovekesh force-pushed the feature/add-support-for-js-modules branch from eb25c11 to f2334bf Compare July 21, 2024 09:39
@thelovekesh thelovekesh changed the title Update node packages and add support for JS modules Add Interactivity API support Jul 21, 2024
@thelovekesh
Copy link
Member

The wp-env package was outdated, and GitHub runners no longer support Docker Compose v1, which caused the error. I have rebased the changes with the main branch.

Comment on lines +12 to +21
const { getAsBooleanFromENV } = require( '@wordpress/scripts/utils' );

const hasExperimentalModulesFlag = getAsBooleanFromENV( 'WP_EXPERIMENTAL_MODULES' );
let scriptConfig, moduleConfig;

if ( hasExperimentalModulesFlag ) {
[ scriptConfig, moduleConfig ] = require( '@wordpress/scripts/config/webpack.config' );
} else {
scriptConfig = require( '@wordpress/scripts/config/webpack.config' );
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const { getAsBooleanFromENV } = require( '@wordpress/scripts/utils' );
const hasExperimentalModulesFlag = getAsBooleanFromENV( 'WP_EXPERIMENTAL_MODULES' );
let scriptConfig, moduleConfig;
if ( hasExperimentalModulesFlag ) {
[ scriptConfig, moduleConfig ] = require( '@wordpress/scripts/config/webpack.config' );
} else {
scriptConfig = require( '@wordpress/scripts/config/webpack.config' );
}
const [ scriptConfig, moduleConfig ] = require( '@wordpress/scripts/config/webpack.config' );

Since you have already enabled the experimental modules in the build script, there is no need to re-determine the logic in the webpack configuration. The getAsBooleanFromENV('WP_EXPERIMENTAL_MODULES') function will always return true in this context.

Comment on lines +95 to +117
let moduleScripts = {};
if ( hasExperimentalModulesFlag ) {
moduleScripts = {
...moduleConfig,
entry: {
'media-text-interactive': path.resolve( process.cwd(), 'assets', 'src', 'js', 'modules', 'media-text-interactive.js' ),
},
output: {
...moduleConfig.output,
path: path.resolve( process.cwd(), 'assets', 'build', 'js', 'modules' ),
filename: '[name].js',
chunkFilename: '[name].js',
},
};
}

const customExports = [ scripts, styles ];

if ( hasExperimentalModulesFlag ) {
customExports.push( moduleScripts );
}

module.exports = customExports;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
let moduleScripts = {};
if ( hasExperimentalModulesFlag ) {
moduleScripts = {
...moduleConfig,
entry: {
'media-text-interactive': path.resolve( process.cwd(), 'assets', 'src', 'js', 'modules', 'media-text-interactive.js' ),
},
output: {
...moduleConfig.output,
path: path.resolve( process.cwd(), 'assets', 'build', 'js', 'modules' ),
filename: '[name].js',
chunkFilename: '[name].js',
},
};
}
const customExports = [ scripts, styles ];
if ( hasExperimentalModulesFlag ) {
customExports.push( moduleScripts );
}
module.exports = customExports;
const mediaTextInteractiveModule = {
...moduleConfig,
entry: {
'media-text-interactive': path.resolve( process.cwd(), 'assets', 'src', 'js', 'modules', 'media-text-interactive.js' ),
},
output: {
...moduleConfig.output,
path: path.resolve( process.cwd(), 'assets', 'build', 'js', 'modules' ),
filename: '[name].js',
chunkFilename: '[name].js',
},
};
module.exports = [ scripts, styles, mediaTextInteractiveModule ];

Comment on lines +59 to +63
public function block_extensions() {

Media_Text_Interactive::get_instance();

}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
public function block_extensions() {
Media_Text_Interactive::get_instance();
}
public function block_extensions() {
Media_Text_Interactive::get_instance();
}

Are these empty lines required here? I guess, it's due to the styling in this code?

Comment on lines +23 to +27
protected function __construct() {

$this->setup_hooks();

}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
protected function __construct() {
$this->setup_hooks();
}
protected function __construct() {
$this->setup_hooks();
}

Copy link
Member

Choose a reason for hiding this comment

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

I think the file name can remain media-text. Multiple core blocks use the interactivity API but do not suffix the block name to indicate the implementation details, such as using the interactivity API.

@divyarajmasani
Copy link
Contributor

@parthnvaswani Please take a looks at the suggestions.

@divyarajmasani
Copy link
Contributor

@parthnvaswani,

To add support for interactive blocks I think only following things are needed,

  • Add @wordpress/interactiviy as dependency
  • block's view.js ( FE script ) will implement the interactivity.
  • Add support in block.json for "interactivity": true.
  • use viewScriptModule attribute to load the script.
  • Add --experimental-modules flag to build and start npm scripts in package.json

Changes in webpack.config, other php classes etc are only to support custom modules, either for block independent functionality or for demonstration on core blocks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update node packages and add support for JS modules
4 participants