WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10.52k stars 4.21k forks source link

Packages: Publish all modules as independent npm packages #3955

Closed Shelob9 closed 6 years ago

Shelob9 commented 6 years ago

Issue Overview and Possible Solution

When creating a block, I start with something like this:

const { __ } = wp.i18n;
const { registerBlockType } = wp.blocks; 
const BlockControls = wp.blocks.BlockControls;
const el = wp.element.createElement;

Strong coupling to global state is consistent with traditional WordPress anti-patterns. But it presents the following problems:

  1. Gutenberg might not be active (3k active installs) or the site might not be on WordPress 5.0 yet (currently ~77% of WordPress sites are not on the current release series.)
  2. Relying on global state makes testing very hard and/or impossible depending on test type/ technology.
  3. I may wish to use these components in my plugin or other type of code that isn't strongly-coupled to WordPress and including them via npm and an import statement is how that should work.

Expected Behavior

$ npm i @wordpress/element
import { createElement } from '@wordpress/element';

Current Behavior

const { createElement } = wp.element;

Tasks

youknowriad commented 6 years ago

There is some work being done making Gutenberg modules available as npm packages. This work has started here https://github.com/WordPress/packages (discussions in the #core-js channel). But all modules should not be used as npm packages because:

Shelob9 commented 6 years ago

@youknowriad I'm glad to hear this is in progress.

You said Some of them require Server Side logic (injected scripts which is a valid concern, but shouldn't blocks to be sufficiently decoupled from that server-side logic that we can inject our own for testing or non-traditional uses, for example Gutenberg for Drupal content editing :)

youknowriad commented 6 years ago

shouldn't blocks to be sufficiently decoupled from that server-side logic that we can inject our own for testing or non-traditional uses, for example Gutenberg for Drupal content editing :)

That's a valid use-case that I care about as well, making Gutenberg (or the core of Gutenberg) a reusable piece. While I think it's important to follow this direction, it's not a priority for the first release.

andreiglingeanu commented 6 years ago

One valid reason for this is i18n. I'd really love to be able to pull @wordpress/i18n package as a standalone thing. I'm building my own private solution for translations (with having a custom Babel plugin, too) and I'd love to not be having to maintain the wp.i18n lib by myself.

Are you guys considering to get at least the small utilities published in NPM, for everyone in the WP community to be able to use & benefit from them?

youknowriad commented 6 years ago

re you guys considering to get at least the small utilities published in NPM, for everyone in the WP community to be able to use & benefit from them?

Yes, we are. Expect more in the future but there are some utilities available right now. Others like components, i18n, etc. will follow https://www.npmjs.com/org/wordpress

andreiglingeanu commented 6 years ago

One more question is, should I count on the technique of putting the lib globally under wp namespace (wp.i18n, wp.blocks, wp.element) as a viable one? Will that be supported with arrival of Gutenberg into the WP core?

If so, I'd like to be able to remove those dependencies from my bundles in the future and just use the globally available ones.

wp.i18n and wp.element appeals to me a lot. This will allow me to build UIs using React without having to ship the lib itself with the plugin.

youknowriad commented 6 years ago

If so, I'd like to be able to remove those dependencies from my bundles in the future and just use the globally available ones.

Yes, I think so. I think we should think about this module by module because making it available in the global means maintaining backwards compatibility for this global. For example the components module, I'm not sure we'd want to make it global in the future.

gziolo commented 6 years ago

We probably will integrate Lerna into this repository and will publish i18n, block, components, element as packages from here. We would use next tag to inform that they are experimental and bump version every time we do a release for a plugin. Is someone interested to work on this?

Shelob9 commented 6 years ago

I made a PHP app that copies all of Gutenberg's built JS/CSS files and publishes them as an npm package

Not sure if this approach makes sense yet or not, but if I can figure out how to get it work with webpack, I can use Travis to run the app's build command every-time a new tag is added to Gutenberg and then publish the same tag to npm.

gziolo commented 6 years ago

We should publish all Gutenberg modules (except edit-post) as individual packages to npm later this month. I’ll be working on it starting next week the latest. I’m not sure if using built files is the way to go in general but might be beneficial in your case. My thinking is that we should publish sources as they are after each new version is tagged.

c-shultz commented 6 years ago

We should publish all Gutenberg modules (except edit-post) as individual packages to npm later this month.

That would be great! That would definitely solve the dependency issues I was running into with unit testing a Jetpack module that depend on Gutenberg's data API.

gziolo commented 6 years ago

A first batch of packages published to npm. See #7027 and:

Successfully published:
 - @wordpress/blob@1.0.0-alpha.0
 - @wordpress/data@1.0.0-alpha.0
 - @wordpress/date@1.0.0-alpha.0
 - @wordpress/deprecated@1.0.0-alpha.0
 - @wordpress/dom@1.0.0-alpha.0
 - @wordpress/element@1.0.0-alpha.0
 - @wordpress/library-export-default-webpack-plugin@1.0.0-alpha.0
 - @wordpress/postcss-themes@1.0.0-alpha.0
lerna success publish finished
gziolo commented 6 years ago

@nerrad, @Shelob9, @andreiglingeanu and @c-shultz - let us know if everything works as expected with the packages that were published. It's very first version so can't guarantee it all works outside of Gutenberg. We did our best to make them independent. Friendly reminder, make sure to exclude them from your build using externals in Webpack config as done in Gutenberg: https://github.com/WordPress/gutenberg/blob/master/webpack.config.js#L168-L177.

nerrad commented 6 years ago

👍 I'm going to try to convert our builds to use the packages some time this week. We're still doing gutenberg implementation on a non-production branch.

Shelob9 commented 6 years ago

@gziolo This is great news. I will try and integrate it into something I'm working on now.

Shelob9 commented 6 years ago

I'm getting an error when importing the data package, or requiring it. I'm using create-react-app.

const data = require( '@wordpress/data' ); and import { data } from '@wordpress/data'; result in Jest throwing an error like this:

/caldera-grid/node_modules/@wordpress/data/src/index.js:4
    import { combineReducers, createStore } from 'redux';
           ^

    SyntaxError: Unexpected token {

I'll figure it out and post a solution.

gziolo commented 6 years ago

I'm getting an error when importing the data package, or requiring it. I'm using create-react-app.

const data = require( '@wordpress/data' ); and import { data } from '@wordpress/data'; result in Jest throwing an error like this:

The issue was on my side. I didn't set main and module inside package.json properly. Fix is ready: #7047. I will publish new version once it is merged.

gziolo commented 6 years ago

I finally was able to publish updated packages. See: #7146. All packages are now available in version 1.0.0-alpha.1. Let me know if that solves your issue.

nerrad commented 6 years ago

I'm still getting the following error after updating all the packages to -alpha.1

/Users/dethier/webprojects/vagrant/www/ee-alpha-testing/wp-content/plugins/event-espresso-core/node_modules/@wordpress/dom/build/index.js:3
    import _Object$defineProperty from 'babel-runtime/core-js/object/define-property';
    ^^^^^^

    SyntaxError: Unexpected token import

      at ScriptTransformer._transformAndBuildScript (node_modules/jest-runtime/build/script_transformer.js:316:17)
      at Object.<anonymous> (node_modules/gutenberg/utils/deprecated.js:14:12)
nerrad commented 6 years ago

ya looks like @wordpress/dom/build/index.js has an import statement when it shouldn't?

nerrad commented 6 years ago

It looks like it may be a babel-transpile bug related to this in dom/src/index.js:

export * from './dom';
gziolo commented 6 years ago

ya looks like @wordpress/dom/build/index.js has an import statement when it shouldn't?

Haha, I see it. It is definitely a bug in Babel. There are two things we still need to attack:

Any thoughts or ideas how to improve this output? For the Gutenberg itself, we should ideally extract all those babel-runtime imports and server as one module. At the moment we have tons of code duplications. Any thoughts?

nerrad commented 6 years ago

Decide what to do about babel-runtime, set it as a dependency would be an easy one, but I'm not sure if this is a common practice in such case.

It seems that is the expectation according to the [https://babeljs.io/docs/plugins/transform-runtime/] docs where they say, "Make sure you include babel-runtime as a dependency".

On the other hand, inlining those Babel runtime files would introduce lots of code duplication between packages.

I'm not sure I follow, isn't it still necessary to have babel-runtime inlined across the packages (since they could be consumed separately from each other)?

youknowriad commented 6 years ago

@gziolo The polyfill issue seems like a generic issue that any npm package could face. Curious how popular packages are solving it?

From what I understand:

Maybe on approach could be to do that as well. Another approach would be to avoid thinking too much about those for each package and aks to require babel-polyfill or something globally? In general we try to avoid loading the babel-polyfill globally like that but I wonder if it's not better to just do it as a separate script instead of duplicating part of it in several modules?

Or

Try the next version of babel-preset-env https://github.com/babel/babel-preset-env/tree/2.0#usebuiltins-usage and specifically the useBuiltIns: 'usage' option

gziolo commented 6 years ago

Migration to Babel 7 is in progress, but we need to fix some configuration issues and fix failing tests. Marking this one as blocked until https://github.com/WordPress/packages/pull/136 gets resolved and Gutenberg upgraded to work with Babel 7.

Shelob9 commented 6 years ago

I'm testing with

import {registerStore} from "@wordpress/data";

    describe( 'wp.data import', () => {
        it( 'registerStore is available',  () => {
            expect(typeof registerStore ).toEqual('function');
        });

        it( 'registerStore works',  () => {
            const initalState = {
                hi: 'Roy'
            };
            expect( registerStore( 'testStore', {
                reducer( state = initalState, action ) {
                    //reduce nothing
                    return state;
                },
            }).getState() ).toEqual(initalState);
        });

    })
});

Both tests are passing :)

gziolo commented 6 years ago

I'm moving plugins module to packages and I noticed the same issue like in dom module:

export * from './components';
export * from './api';

transpiles to commonjs with the unexpected:

import _Object$defineProperty from 'babel-runtime/core-js/object/define-property';
import _Object$keys from 'babel-runtime/core-js/object/keys';
gziolo commented 6 years ago

7832 upgrades Babel to v7 which will resolve all the issues we have with the transpiled code 🎉

gziolo commented 6 years ago

I published last of the planned packages with https://github.com/WordPress/gutenberg/commit/e1811b2082d944dc68bd4c4f048e71cbbdcf6550

All packages can be downloaded from npm using the stable version. Let's open a follow up issues when they are discovered and iterate on the current setup independently.