Automattic / jetpack

Security, performance, marketing, and design tools — Jetpack is made by WordPress experts to make WP sites safer and faster, and help you grow your traffic.
https://jetpack.com/
Other
1.59k stars 798 forks source link

`@automattic/jetpack-connection` npm package is not usable in a custom plugin with `@wordpress/scripts` #39532

Open ecgan opened 1 month ago

ecgan commented 1 month ago

Impacted plugin

None / Other

Quick summary

I am developing a custom WooCommerce plugin. The plugin uses https://www.npmjs.com/package/@wordpress/scripts and https://www.npmjs.com/package/@wordpress/dependency-extraction-webpack-plugin.

When I try to use https://www.npmjs.com/package/@automattic/jetpack-connection in the plugin to get Jetpack connection status (using select( CONNECTION_STORE_ID ).getConnectionStatus() as shown in the npm package readme.md), I'm seeing a bunch of errors in the terminal when I build the plugin. Here are some examples:

WARNING in ./node_modules/@automattic/jetpack-config/src/index.js 6:17-43
Module not found: Error: Can't resolve 'jetpackConfig' in '/Users/engchin/Code/woocommerce/woocommerce-analytics/node_modules/@automattic/jetpack-config/src'
 @ ./node_modules/@automattic/jetpack-api/index.jsx 1:0-80 92:8-24 93:25-41
 @ ./node_modules/@automattic/jetpack-connection/components/connect-user/index.jsx 1:0-46 35:3-36:26
 @ ./node_modules/@automattic/jetpack-connection/index.jsx 25:0-67 25:0-67
 @ ./js/src/screens/order-attribution/use-jetpack-connection.ts 2:0-69 8:31-50
 @ ./js/src/screens/order-attribution/order-attribution.tsx 10:0-64 19:6-26
 @ ./js/src/screens/order-attribution/index.ts 1:0-66 1:0-66
 @ ./js/src/index.js 21:0-59 74:15-31

1 warning has detailed information that is not shown.
Use 'stats.errorDetails: true' resp. '--stats-error-details' to show it.

ERROR in ./node_modules/@automattic/jetpack-connection/components/connect-button/index.jsx 41:2
Module parse failed: Unexpected token (41:2)
You may need an appropriate loader to handle this file type, currently no loaders are configured to process this file. See https://webpack.js.org/concepts#loaders
| 
|       return (
>               <>
|                       { ( ! isRegistered || ! isUserConnected ) && (
|                               <ActionButton
 @ ./node_modules/@automattic/jetpack-connection/index.jsx 23:0-71 23:0-71
 @ ./js/src/screens/order-attribution/use-jetpack-connection.ts 2:0-69 8:31-50
 @ ./js/src/screens/order-attribution/order-attribution.tsx 10:0-64 19:6-26
 @ ./js/src/screens/order-attribution/index.ts 1:0-66 1:0-66
 @ ./js/src/index.js 21:0-59 74:15-31

ERROR in ./node_modules/@automattic/jetpack-connection/components/connect-screen/basic/index.tsx 6:7
Module parse failed: Unexpected token (6:7)
You may need an appropriate loader to handle this file type, currently no loaders are configured to process this file. See https://webpack.js.org/concepts#loaders
| import ConnectScreenVisual from './visual';
| 
> export type Props = {
|       // API root
|       apiRoot: string;
 @ ./node_modules/@automattic/jetpack-connection/index.jsx 20:0-77 20:0-77
 @ ./js/src/screens/order-attribution/use-jetpack-connection.ts 2:0-69 8:31-50
 @ ./js/src/screens/order-attribution/order-attribution.tsx 10:0-64 19:6-26
 @ ./js/src/screens/order-attribution/index.ts 1:0-66 1:0-66
 @ ./js/src/index.js 21:0-59 74:15-31

I looked into the code in https://www.npmjs.com/package/@automattic/jetpack-connection package and they seem to be the original source code with .jsx, .ts and .tsx files. There are no build or build-module directories. I believe this is the cause of the problem.

As a comparison, if we look into the code in https://www.npmjs.com/package/@wordpress/components npm package, we can see there are build or build-module directories, and they work out of the box with no issues in my plugin.

Steps to reproduce

  1. Create a plugin with https://www.npmjs.com/package/@wordpress/scripts and https://www.npmjs.com/package/@wordpress/dependency-extraction-webpack-plugin.
  2. Use https://www.npmjs.com/package/@automattic/jetpack-connection to get Jetpack connection status. Below is a hook code example that you can use in a component:
import { useSelect } from '@wordpress/data';
import { CONNECTION_STORE_ID } from '@automattic/jetpack-connection';

export const useJetpackConnection = () => {
    return useSelect( ( select ) => {
        return {
            connectionStatus:
                select( CONNECTION_STORE_ID ).getConnectionStatus(),
        };
    }, [] );
};
  1. Build the plugin with npm run build or npm run start.

A clear and concise description of what you expected to happen.

Build should be successful with no warnings and errors.

What actually happened

There are build warnings and errors as mentioned above.

Impact

One

Available workarounds?

There is no user impact

If the above answer is "Yes...", outline the workaround.

No response

Platform (Simple and/or Atomic)

No response

Logs or notes

No response

anomiex commented 1 month ago

I looked into the code in https://www.npmjs.com/package/@automattic/jetpack-connection package and they seem to be the original source code with .jsx, .ts and .tsx files. There are no build or build-module directories. I believe this is the cause of the problem.

Partially so. The default configuration for most JS build toolchains is to not transpile code from node_modules/, assuming that most packages will provide built JS rather than source jsx or TypeScript. Adding a build step to js-packages/connection to conform to this expectation is one possible fix. Reconfiguring your tooling to transpile this package is another.

Chances are we should have the package conform to the expectation, I just wanted to be clear about what the problem is.

Then there's the question of whether we should use tsc to simply process the sources, assuming they'll be bundled by the consumer anyway, or webpack to generate minified bundles. I'd probably lean towards the former.

As a comparison, if we look into the code in https://www.npmjs.com/package/@wordpress/components npm package, we can see there are build or build-module directories, and they work out of the box with no issues in my plugin.

34118 may be instructive as to the sort of changes needed in .gitattributes, .gitignore, composer.json, and package.json to do this within the monorepo. Unfortunately there's a bunch of other cleanup to the package happening in that PR too.

Robertght commented 1 month ago

I'm going to mark this as Triaged on our board, but let us know if you need help testing anything related to this.