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.

Sign up for GitHub

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

Protect Components: Use @wordpress/dataviews/wp import path #41510

Draft
wants to merge 17 commits into
base: add/protect/core
Choose a base branch
from
Draft

Protect Components: Use @wordpress/dataviews/wp import path #41510

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
9633458
Init project branch
nateweller Nov 14, 2024
4644928
Init project branch
nateweller Nov 14, 2024
33b77fc
minor adjustments
nateweller Jan 31, 2025
4b13484
Init project branch
nateweller Nov 14, 2024
0de3241
Init project branch
nateweller Nov 14, 2024
09b9e2d
Import from @wordpress/dataviews/wp instead of @wordpress/dataviews
nateweller Feb 3, 2025
d3941a0
Add missing deps to .pnpmfile.cjs
nateweller Feb 3, 2025
df4416d
Update pnpm-lock.yaml
nateweller Feb 3, 2025
1791902
Change target version for missing deps to @wordpress/dataviews 4.12.0
nateweller Feb 3, 2025
0bbb378
Only target the /wp import path for transpilation
nateweller Feb 3, 2025
4b590d0
Add more missing deps to pnpmfile
nateweller Feb 3, 2025
42d2496
Fix transpilation of dataviews for Jetpack Protect
nateweller Feb 4, 2025
8d55b07
changelog
nateweller Feb 4, 2025
0cd0079
Regenerate pnpm.lock
nateweller Feb 5, 2025
d5d4226
Update .pnpmfile.cjs
nateweller Feb 5, 2025
332a427
Update pnpm-lock.yaml
nateweller Feb 5, 2025
c97a1b2
Remove unintentional changes introduced during rebase
nateweller Feb 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions .pnpmfile.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,36 @@ function fixDeps( pkg ) {
pkg.peerDependencies[ '@babel/runtime' ] = '^7';
}

// Missing deps.
// https://github.com/WordPress/gutenberg/issues/67864
if ( pkg.name === '@wordpress/dataviews' ) {
for ( const dep of [
Copy link
Member

Choose a reason for hiding this comment

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

How did you compile this list? Was it by looking at the errors during building?

Copy link
Contributor

@anomiex anomiex Feb 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I don't remember how I got it for WordPress/gutenberg#67864. 😀 Possibly that, or possibly by grepping build-wp/index.js for import statements and then removing the ones that are already dependencies of @wordpress/dataviews.

'change-case',
'colord',
'date-fns',
'deepmerge',
'@emotion/cache',
'@emotion/css',
'@emotion/react',
'@emotion/styled',
'@emotion/utils',
'fast-deep-equal',
'@floating-ui/react-dom',
'framer-motion',
'highlight-words-core',
'is-plain-object',
'memize',
'react-dom',
'@use-gesture/react',
'use-memo-one',
'uuid',
'@wordpress/date',
'@wordpress/hooks',
] ) {
pkg.optionalDependencies[ dep ] = '*';
}
}

// Missing dep or peer dep.
// https://github.com/actions/toolkit/issues/1684
if (
Expand Down
103 changes: 83 additions & 20 deletions pnpm-lock.yaml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions projects/js-packages/components/changelog/use-dataviews-wp-import
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: changed

Implement the WP-specific import path for the DataViews component.
2 changes: 1 addition & 1 deletion projects/js-packages/components/components/scan-report/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
type Field,
DataViews,
filterSortAndPaginate,
} from '@wordpress/dataviews';
} from '@wordpress/dataviews/wp';
import { __, _n } from '@wordpress/i18n';
import { Icon } from '@wordpress/icons';
import { useCallback, useMemo, useState } from 'react';
Expand Down
2 changes: 1 addition & 1 deletion projects/js-packages/components/components/threats-data-views/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {
type View,
DataViews,
filterSortAndPaginate,
} from '@wordpress/dataviews';
} from '@wordpress/dataviews/wp';
import { dateI18n } from '@wordpress/date';
import { __ } from '@wordpress/i18n';
import { Icon } from '@wordpress/icons';
Expand Down
4 changes: 4 additions & 0 deletions projects/plugins/protect/changelog/protect-update-compile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: changed

Update build to support using DataViews from @automattic/jetpack-components.
47 changes: 45 additions & 2 deletions projects/plugins/protect/webpack.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,56 @@ module.exports = [
* @see https://github.com/Automattic/jetpack/issues/39907
*/
jetpackWebpackConfig.TranspileRule( {
includeNodeModules: [ '@wordpress/dataviews/' ],
includeNodeModules: [ '@wordpress/dataviews/build-wp/' ],
babelOpts: {
configFile: false,
plugins: [
[
require.resolve( '@automattic/babel-plugin-replace-textdomain' ),
{ textdomain: 'jetpack-protect' },
{
textdomain: 'jetpack-protect',
functions: {
__: 1,
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need the functions object here?

Was the compiling manual? If yes, would that mean updating things here if the core DataViews package adds more i18n functions in its code?

Copy link
Contributor

Choose a reason for hiding this comment

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

The real problem is that build-wp/index.js isn't preserving the names of the @wordpress/i18n functions. Instead it does a lot of aliasing like import { __ as __9 } from '@wordpress/i18n';.

It turns out that Webpack as used in this particular PR currently winds up undoing all the bad import aliasing, going back to the correct __ name so GlotPress's use of wp i18n will pick up the strings from the bundle. But @automattic/babel-plugin-replace-textdomain (which runs long before Webpack fixes it) needs this to know to inject the correct textdomain into all the aliased calls, because the translations won't be in Core's default domain.

Someday, when Core distributes @wordpress/dataviews like it does @wordpress/components and other packages, none of this will be needed because both the script and translations will come from Core instead of having to be bundled with the plugin. It's unfortunate that we're currently in this situation where @wordpress/dataviews is too unstable for Core to provide but everyone wants to start using it anyway.

__1: 1,
__2: 1,
__3: 1,
__4: 1,
__5: 1,
__6: 1,
__7: 1,
__8: 1,
__9: 1,
__10: 1,
__11: 1,
__12: 1,
__13: 1,
__14: 1,
__15: 1,
__16: 1,
__17: 1,
__18: 1,
__19: 1,
__20: 1,
__21: 1,
__22: 1,
__23: 1,
__24: 1,
__25: 1,
__26: 1,
__27: 1,
__28: 1,
__29: 1,
__30: 1,
_x: 2,
_x1: 2,
_x2: 2,
_x3: 2,
_x4: 2,
_x5: 2,
_n: 3,
_nx: 4,
},
},
],
],
},
Expand Down
Loading