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

Packages: Move element to packages maintained by Lerna #6756

Merged
merged 2 commits into from
May 15, 2018
+67 −26
Merged

Packages: Move element to packages maintained by Lerna #6756

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f1a9e7b
Packages: Move element to packages maintained by Lerna
gziolo May 15, 2018
b333b22
Packages: Add React keyword to the description
gziolo May 15, 2018
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
2 changes: 1 addition & 1 deletion components/autocomplete/test/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@
* External dependencies
*/
import { mount } from 'enzyme';
import { Component } from '../../../element';
import { noop } from 'lodash';

/**
* WordPress dependencies
*/
import { Component } from '@wordpress/element';
import { keycodes } from '@wordpress/utils';

/**
Expand Down
6 changes: 5 additions & 1 deletion components/higher-order/with-focus-return/test/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,11 @@
* External dependencies
*/
import { shallow, mount } from 'enzyme';
import { Component } from '../../../../element';

/**
* WordPress dependencies
*/
import { Component } from '@wordpress/element';

/**
* Internal dependencies
Expand Down
4 changes: 2 additions & 2 deletions data/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ registerStore( 'my-shop', {

### Higher-Order Components

A higher-order component is a function which accepts a [component](https://github.com/WordPress/gutenberg/tree/master/element) and returns a new, enhanced component. A stateful user interface should respond to changes in the underlying state and updates its displayed element accordingly. WordPress uses higher-order components both as a means to separate the purely visual aspects of an interface from its data backing, and to ensure that the data is kept in-sync with the stores.
A higher-order component is a function which accepts a [component](https://github.com/WordPress/gutenberg/tree/master/packages/element) and returns a new, enhanced component. A stateful user interface should respond to changes in the underlying state and updates its displayed element accordingly. WordPress uses higher-order components both as a means to separate the purely visual aspects of an interface from its data backing, and to ensure that the data is kept in-sync with the stores.

#### `withSelect( mapSelectToProps: Function ): Function`

Expand Down Expand Up @@ -239,7 +239,7 @@ const SaleButton = withDispatch( ( dispatch, ownProps ) => {

The data module shares many of the same [core principles](https://redux.js.org/introduction/three-principles) and [API method naming](https://redux.js.org/api-reference) of [Redux](https://redux.js.org/). In fact, it is implemented atop Redux. Where it differs is in establishing a modularization pattern for creating separate but interdependent stores, and in codifying conventions such as selector functions as the primary entry point for data access.

The [higher-order components](#higher-order-components) were created to complement this distinction. The intention with splitting `withSelect` and `withDispatch` — where in React Redux they are combined under `connect` as `mapStateToProps` and `mapDispatchToProps` arguments — is to more accurately reflect that dispatch is not dependent upon a subscription to state changes, and to allow for state-derived values to be used in `withDispatch` (via [higher-order component composition](https://github.com/WordPress/gutenberg/tree/master/element#compose)).
The [higher-order components](#higher-order-components) were created to complement this distinction. The intention with splitting `withSelect` and `withDispatch` — where in React Redux they are combined under `connect` as `mapStateToProps` and `mapDispatchToProps` arguments — is to more accurately reflect that dispatch is not dependent upon a subscription to state changes, and to allow for state-derived values to be used in `withDispatch` (via [higher-order component composition](https://github.com/WordPress/gutenberg/tree/master/packages/element#compose)).

Specific implementation differences from Redux and React Redux:

Expand Down
2 changes: 1 addition & 1 deletion docs/block-api/block-edit-save.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ save() {
```
{% end %}

For most blocks, the return value of `save` should be an [instance of WordPress Element](https://github.com/WordPress/gutenberg/blob/master/element/README.md) representing how the block is to appear on the front of the site.
For most blocks, the return value of `save` should be an [instance of WordPress Element](https://github.com/WordPress/gutenberg/blob/master/packages/element/README.md) representing how the block is to appear on the front of the site.

_Note:_ While it is possible to return a string value from `save`, it _will be escaped_. If the string includes HTML markup, the markup will be shown on the front of the site verbatim, not as the equivalent HTML node content. If you must return raw HTML from `save`, use `wp.element.RawHTML`. As the name implies, this is prone to [cross-site scripting](https://en.wikipedia.org/wiki/Cross-site_scripting) and therefore is discouraged in favor of a WordPress Element hierarchy whenever possible.

Expand Down
2 changes: 1 addition & 1 deletion docs/blocks/generate-blocks-with-wp-cli.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ add_action( 'init', 'movie_block_init' );
var registerBlockType = wp.blocks.registerBlockType;
/**
* Returns a new element of given type. Element is an abstraction layer atop React.
* @see https://github.com/WordPress/gutenberg/tree/master/element#element
* @see https://github.com/WordPress/gutenberg/tree/master/packages/element#element
*/
var el = wp.element.createElement;
/**
Expand Down
4 changes: 2 additions & 2 deletions docs/blocks/writing-your-first-block-type.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ add_action( 'init', 'gutenberg_boilerplate_block' );
Note the two script dependencies:

- __`wp-blocks`__ includes block type registration and related functions
- __`wp-element`__ includes the [WordPress Element abstraction](https://github.com/WordPress/gutenberg/tree/master/element) for describing the structure of your blocks
- __`wp-element`__ includes the [WordPress Element abstraction](https://github.com/WordPress/gutenberg/tree/master/packages/element) for describing the structure of your blocks

## Registering the Block

Expand Down Expand Up @@ -80,7 +80,7 @@ registerBlockType( 'gutenberg-boilerplate-esnext/hello-world-step-01', {
```
{% end %}

Once a block is registered, you should immediately see that it becomes available as an option in the editor inserter dialog, using values from `title`, `icon`, and `category` to organize its display. You can choose an icon from any included in the built-in [Dashicons icon set](https://developer.wordpress.org/resource/dashicons/), or provide your own by assigning the value of `icon` as a [WordPress Element](https://github.com/WordPress/gutenberg/tree/master/element) element or component.
Once a block is registered, you should immediately see that it becomes available as an option in the editor inserter dialog, using values from `title`, `icon`, and `category` to organize its display. You can choose an icon from any included in the built-in [Dashicons icon set](https://developer.wordpress.org/resource/dashicons/), or provide your own by assigning the value of `icon` as a [WordPress Element](https://github.com/WordPress/gutenberg/tree/master/packages/element) element or component.

A block name must be prefixed with a namespace specific to your plugin. This helps prevent conflicts when more than one plugin registers a block with the same name.

Expand Down
6 changes: 3 additions & 3 deletions package-lock.json
View file
Open in desktop

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

2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
"@wordpress/dom-ready": "1.0.4",
"@wordpress/hooks": "1.1.6",
"@wordpress/i18n": "1.1.0",
"@wordpress/is-shallow-equal": "1.0.1",
Copy link
Contributor

Choose a reason for hiding this comment

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

Any particular reason for this upgrade?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, there were some issues with the package outside of Gutenberg and it wasn't bumped in Gutenberg. See: WordPress/packages#124.
We can do it separately.

Copy link
Contributor

Choose a reason for hiding this comment

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

No, I'm fine keeping it, just curious

"@wordpress/is-shallow-equal": "1.0.2",
"@wordpress/url": "1.1.0",
"@wordpress/wordcount": "1.0.2",
"classnames": "2.2.5",
Expand Down
15 changes: 12 additions & 3 deletions element/README.md → packages/element/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
Element
=======
# @wordpress/element

Element is, quite simply, an abstraction layer atop [React](https://reactjs.org/).

Expand All @@ -14,7 +13,15 @@ On the `wp.element` global object, you will find the following, ordered roughly
- [`createElement`](https://reactjs.org/docs/react-api.html#createelement)
- [`render`](https://reactjs.org/docs/react-dom.html#render)

## Example
## Installation

Install the module

```bash
npm install @wordpress/element@next --save
```

## Usage

Let's render a customized greeting into an empty element:

Expand Down Expand Up @@ -62,3 +69,5 @@ If you've configured [Babel](http://babeljs.io/) for your project, you can opt i
]
}
```

<br/><br/><p align="center"><img src="https://s.w.org/style/images/codeispoetry.png?1" alt="Code is Poetry." /></p>
30 changes: 30 additions & 0 deletions packages/element/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
{
"name": "@wordpress/element",
"version": "0.0.1",
"description": "Element React module for WordPress",
"author": "WordPress",
"license": "GPL-2.0-or-later",
"keywords": [
"wordpress",
"element",
"react"
],
"homepage": "https://github.com/WordPress/gutenberg/tree/master/packages/element/README.md",
"repository": {
"type": "git",
"url": "https://github.com/WordPress/gutenberg.git"
},
"bugs": {
"url": "https://github.com/WordPress/gutenberg/issues"
},
"main": "src/index.js",
"dependencies": {
"@wordpress/is-shallow-equal": "1.0.2",
"lodash": "4.17.5",
"react": "16.3.2",
"react-dom": "16.3.2"
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this mean, we can remove react and react-dom from the root level package.json?

Copy link
Member Author

Choose a reason for hiding this comment

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

Technically we can, but they won't be tracked in package-json.lock. In the previous PR we decided to keep them in sync with the root level package.json. We can revisit later. Personally, I would prefer to have it working with lock files and have only one place to include them.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's not really important since it's just for test. But I agree it's a concern for other dependencies that are bundled and I think we should have a separate lock per module.

Copy link
Member Author

@gziolo gziolo May 15, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

You are right, I forgot about it. As far as I remember, those local lock files, don't really work when using lerna bootstrap --hoist. @ntwb - or do they and we disabled to follow semantic versioning?

Copy link
Member

Choose a reason for hiding this comment

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

Hmmm, I've not got an accurate recollection, there was some trouble I think.

That said, https://github.com/WordPress/packages do not have lock files, why would the https://github.com/WordPress/gutenberg require lock files.

tl;dr: Lock files are for apps, not packages or modules

Copy link
Contributor

Choose a reason for hiding this comment

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

I understand your point but I think I disagree. The difference between "app" and "packages" is superficial. What's the app when your whole app is a collection of packages? How do you guarantee an npm install won't inadvertently break your application?

Copy link
Member

Choose a reason for hiding this comment

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

You can't which is one of my points in that if you look down an "app" or "package" with specific package versions using a lock file then what is the point of SemVer, you may as well release everything as a major release and never worry about patch or minor releases for an "app" or "package" that is depended upon

Copy link
Contributor

@youknowriad youknowriad May 15, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

So you're ok with merging a PR and than your automatic CI job releasing your packages/app break because it goes with the wrong package you never tested?

I'm not. I see where you're coming from but that's true when dependencies get bundled together to build a single bundle which is not the case in our setup, each package is an app that generates its own script. We need predictability.

Copy link
Member

Choose a reason for hiding this comment

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

On a "package" side of things I am happy to let SemVer do what it was designed to do.

On an "app" I'm with you 100% and everything should be locked down.

Anything in between is tricky, which is where we are at here...

If the Gutenberg packages are released using SemVer then using an explicit locked down version e.g. 1.0.0 and not ~1.0.0 or ^1.0.0 of that package for Gutenberg with a lock file will work perfectly for Gutenberg as the only time a dependancy is bumped is when it is manually bumped which would include testing that the version bump works as expected.

Individual packages are then free to be used in the wider community and will continue to adhere to SemVer allowing other packages to depend upon them and update as they see fit per their own SemVer policy.

Anyways, that's my take, should probably save this discussion for the follow up PR @gziolo has planned 😄

Copy link
Contributor

Choose a reason for hiding this comment

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

But I'm still not sure I follow you. There's no difference between an "app" and a "package". All Gutenberg packages are "apps" because they're all released as such as WordPress scripts.

Gutenberg will never "require" or depend on Gutenberg packages because they're built in the same repository. That may change (not certain) when the merge happens but until then, all the packages are apps, there's no higher-level package-lock.json where they can be locked.

},
"publishConfig": {
"access": "public"
}
}
0 element/index.js → packages/element/src/index.js
View file
Open in desktop
File renamed without changes.
0 element/index.native.js → packages/element/src/index.native.js
View file
Open in desktop
File renamed without changes.
0 element/serialize.js → packages/element/src/serialize.js
View file
Open in desktop
File renamed without changes.
0 element/test/index.js → packages/element/src/test/index.js
View file
Open in desktop
File renamed without changes.
0 element/test/serialize.js → packages/element/src/test/serialize.js
View file
Open in desktop
File renamed without changes.
6 changes: 3 additions & 3 deletions test/unit/jest.config.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
{
"rootDir": "../../",
"collectCoverageFrom": [
"(blocks|components|editor|element|data|utils|edit-post|viewport|plugins|core-data|core-blocks)/**/*.js",
"(blocks|components|editor|data|utils|edit-post|viewport|plugins|core-data|core-blocks)/**/*.js",
"packages/**/*.js"
],
"moduleNameMapper": {
"@wordpress\\/(blocks|components|editor|element|data|utils|edit-post|viewport|plugins|core-data|core-blocks)": "$1",
"@wordpress\\/(date)": "packages/$1/src"
"@wordpress\\/(blocks|components|editor|data|utils|edit-post|viewport|plugins|core-data|core-blocks)": "$1",
"@wordpress\\/(date|element)": "packages/$1/src"
},
"preset": "@wordpress/jest-preset-default",
"setupFiles": [
Expand Down
12 changes: 5 additions & 7 deletions test/unit/setup-wp-aliases.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,9 @@ global.wp = {
},
};

[
'element',
'blocks',
].forEach( ( entryPointName ) => {
Object.defineProperty( global.wp, entryPointName, {
get: () => require( entryPointName ),
} );
Object.defineProperty( global.wp, 'element', {
get: () => require( 'packages/element' ),
Copy link
Member

Choose a reason for hiding this comment

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

How is it that packages/element refers to the packages/element folder, when it's not being defined as a path (and even if it were, required from test/unit)?

Copy link
Member Author

Choose a reason for hiding this comment

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

Jest resolves those paths starting from the root dir, that's why it works.

Our goal is to get rid of this workaround completely when we have all modules moved to packages and we enforce using createElement in files using JSX - related commit 69af0a4 from #6828.

The same applies to wb.blocks which is here temporarily until we remove deprecations which use this global explicitly.

} );
Object.defineProperty( global.wp, 'blocks', {
get: () => require( 'blocks' ),
} );
2 changes: 1 addition & 1 deletion webpack.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ const entryPointNames = [
'blocks',
'components',
'editor',
'element',
'utils',
'data',
'viewport',
Expand All @@ -83,6 +82,7 @@ const entryPointNames = [

const gutenbergPackages = [
'date',
'element',
];

const wordPressPackages = [
Expand Down