Build: Move date module to packages maintained by Lerna #6658

gziolo · 2018-05-09T12:35:56Z

Description

Related issues: #3955, #6594.

This PR tries to provide Lerna setup similar to what we have in WordPress/packages. This would allow publishing source code of individual modules to npm to make them available for all plugin developers.

I started small with the date module to start the discussion.

The version number would be maintained in Lerna config instead of package.json. So the idea I had is that every time we publish a new version of the plugin to .org, we publish also the same version of npm packages. We will have to revisit after 5.0 mission is accomplished.

In the first iteration, we would release date with 2.8-alpha version as a development release. Once it is confirmed it works as expected, and we have build and build-module distributions we could switch to regular releases.

Issues discovered

We are using package-lock.json for Gutenberg. I disabled it for date package because it isn't required there at all. However when using --hoist flag with Lerna (to avoid duplicated node_modules) I noticed that package-lock.json doesn't get dependencies from packages included in the lock flag. To mitigate it I had to duplicate the same dependencies in both package and root package.json file. Question is if we should maintain it in both places or is fine to have it outside of lock file control?

How has this been tested?

Manually:

Removed node_modules folder.
npm install
npm test
npm run dev
npm run build
npm run package-plugin

New commands introduced:

npm run publish:check
npm run publish:dev
npm run publish:prod

It mirros what we have in WordPress/packages.

Types of changes

Refactoring.

Checklist:

My code is tested.
My code follows the WordPress code style.
My code follows the accessibility standards.
My code has proper inline documentation.

youknowriad · 2018-05-09T12:48:42Z

components/date-time/date.js

+/**
+ * WordPress dependencies
+ */
+import { moment } from '@wordpress/date';


The usage of the regular moment was intentional here. The idea is that we don't need any locale set, we just use moment as an utility function without global locale. (this avoids a dependency between components and date which I think would be nice to keep independent.)

Reverting those changes 👍
I wasn't aware of this. It still would be useful to wrap those moment calls with some higher level API to make it possible to replace moment with something else as soon as there is good alternative for it :)

tofumatt

The exporting of moment from our package strikes me as strange and I'm finding it hard to separate out the package change from it conceptually 😅

The issue with losing package locking is just for our internal dependencies, which are in the repo already, right? If that's the case it's probably fine. 🤷‍♂️

tofumatt · 2018-05-09T12:40:48Z

components/date-time/date.js

+/**
+ * WordPress dependencies
+ */
+import { moment } from '@wordpress/date';


I'm confused about the utility of importing this package from @wordpress/date when it's already available as a regular dependency. It seems like more stuff we have to manage to export the same API (and if we aren't exposing moment's exact API we should maybe call it something different). Why do this?

I agree with this, the idea is that it absorbs the WP locale and timezone settings. But the downside is that this forces us somehow to stay compatible with the current moment version. We should definitely try to remove moment from the date module entirely, but let's leave it for a separate PR.

tofumatt · 2018-05-09T12:44:12Z

webpack.config.js

@@ -123,7 +127,12 @@ const config = {
 			memo[ name ] = `./${ path }`;
 			return memo;
 		}, {} ),
-		packageNames.reduce( ( memo, packageName ) => {
+		gutenbergPackages.reduce( ( memo, path ) => {


Any reason to rename packageName to path? It's still just using the packageName and I like the explicitness of the variable name. Plus it will hopefully discourage someone from including more nested paths.

I guess it's copy and paste thing. Will fix :)

youknowriad · 2018-05-09T13:01:24Z

webpack.config.js

-		packageNames.reduce( ( memo, packageName ) => {
+		gutenbergPackages.reduce( ( memo, path ) => {
+			const name = camelCaseDash( path );
+			memo[ name ] = `./packages/${ path }`;


should we add an src folder here (and move the files) ./packages/${ path }/src to match the packages repository setup?

I have it locally, need to push. I thought the same when I saw diff on GitHub :)

youknowriad · 2018-05-09T13:03:00Z

packages/date/package.json

+	"publishConfig": {
+		"access": "public"
+	}
+}


I believe this is lacking the main/module configs to make the modules ready for npm. This would probably have an impact on the webpack config (separate dist folder) than the one currently used for the remaining modules

Which brings us back to having a dedicated build folder per package maybe 🤷‍♂️

It seems like we need to have independent build folders for npm, because we use very specific Webpack config which does some additional work like exposing wp. globals. We also need to expose two version:

es5 friendly

import/export friendly

I've always found a dedicated build folder much easier to grok when I first explore a package

gziolo · 2018-05-09T13:27:35Z

The issue with losing package locking is just for our internal dependencies, which are in the repo already, right? If that's the case it's probably fine.

it's for moment and moment-timezone, but it will become a wider issue when we move more packages. We need to provide the list of dependencies for every package we publish. To give some examples, for element it will be react and react-dom, for data it will be redux. For components the list will be long, and so on :)

ntwb · 2018-05-09T13:33:52Z

I noticed that package-lock.json doesn't get dependencies from packages included in the lock flag. To mitigate it I had to duplicate the same dependencies in both package and root package.json file. Question is if we should maintain it in both places or is fine to have it outside of lock file control?

Maintaining both, manually, if that is what is required would be my suggestion, the latest npm releases heavily depend on lock files when installing, so either no lock file, or an up to date lock file is pretty much required.

gziolo · 2018-05-09T13:47:10Z

Let's try to keep lock file as it is and maintain duplicates. If this is too much hassle we can always use package linter we are about to roll out for packages repository :)
I think Lerna also detects if versions of the same library don't match.

ntwb · 2018-05-09T13:47:30Z

lerna.json

+	"lerna": "2.11.0",
+	"commands": {
+		"publish": {
+			"message": "chore(release): publish %s"


If the packages are going to be independently released as they are currently with the packages repo the %s here will be ignored and should be dropped here, see https://github.com/lerna/lerna#--message--m-msg

I thought we keep one version aligned with Gutenberg release version. I need to collect all my thoughts and move to description :)
So the idea I had is that every time we publish a new version of the plugin to .org, we publish also the same version of npm packages. We will have to revisit after 5.0 mission is accomplished.

Yes, makes sense 👍

On the %s issue, did you run lerna init with the --independent flag? If so %s will be ignored, I'll take a closer look tomorrow, its a pretty minor thing that we will see if it is or is not when lerna publish is run for th first time 😏

lerna init only :)

ntwb · 2018-05-09T14:06:11Z

Now's a good time to add a CHANGELOG.md file for the date package, see WordPress/packages#118

youknowriad · 2018-05-09T14:35:46Z

packages/date/src/index.js

+	return settings;
+}
+
+function setupWPTimezone() {


I'd love to get rid of moment config entirely and just export simple functions but let's do this separately. The date module is still usable in a generic way using get/setSettings at the moment

We can now remove all those date related logic from the default Jest config. I will take care of it separately.

youknowriad · 2018-05-09T14:37:15Z

package.json

 		"pot-to-php": "./bin/pot-to-php.js",
+		"publish:check": "lerna updated",
+		"publish:dev": "lerna publish --npm-tag next",
+		"publish:prod": "lerna publish",


Adding docs to the RELEASE.md would be good (or contributing) to explain how to use these things :), could be done later once the config is more battle-tested.

Copied https://github.com/WordPress/gutenberg/blob/37a722322327ee45f0510e0c7fa9a2ec2f793db2/CONTRIBUTING.md#releasing-packages from the existing docs in WordPress/packages :)
Props to @youknowriad and @pento 🙇

youknowriad

I do think we should address the npm builds instead of just publishing the sources. I can tackle that separately though.

Also let's give this some visibility as it's important @aduth @mtias @mcsf

aduth · 2018-05-09T23:28:39Z

This PR tries to provide Lerna setup similar to what we have in WordPress/packages. This would allow publishing source code of individual modules to npm to make them available for all plugin developers.

What process do you propose we adopt for cutting releases?

gziolo · 2018-05-10T09:47:00Z

What process do you propose we adopt for cutting releases?

@aduth, I briefly described it as follows:

So the idea I had is that every time we publish a new version of the plugin to .org, we publish also the same version of npm packages. We will have to revisit after 5.0 mission is accomplished.

In the first iteration, we would release date with 2.8-alpha version as a development release. Once it is confirmed it works as expected, and we have build (es5) and build-module (esnext with import/export) distributions we could switch to regular releases.

To expand on it, once this PR is merged, we publish 2.8-alpha for @wordpress/date. Then we repeat the same steps for all modules that don't depend on globals. I will take care of it. At the same time, @youknowriad will check how to expose build and build-module versions in a similar fashion to what we do in WordPress/packages - (ideally we share the same script).
When we release v2.9 of Gutenberg, we also publish 2.9 version of npm modules (alpha or stable depending on the progress). We repeat the steps every time a new version of Gutenberg is released. We continue until Gutenberg is merged to core and 5.0 is out, then we decide what's next.

gziolo · 2018-05-10T11:14:24Z

Now's a good time to add a CHANGELOG.md file for the date package, see WordPress/packages#118

See: lerna/lerna#850 (comment)

--conventional-commits is currently only enabled when you use the "independent" versioning mode, not the default "fixed" versioning (the version field of lerna.json).

Only per-package CHANGELOG.md files are currently updated, not the root.

Enabling --conventional-commits from lerna.json should look something like this:

Let's test it in WordPress/packages first and see how it goes. I don't feel like independent makes a lot of sense when individual packages from Gutenberg depend on each other. It might be very confusing to use them otherwise.

aduth · 2018-05-10T19:58:08Z

So the idea I had is that every time we publish a new version of the plugin to .org, we publish also the same version of npm packages. We will have to revisit after 5.0 mission is accomplished.

So not SemVer ?

tofumatt · 2018-05-10T20:42:06Z

Not using SemVer is a 👎 from me; it makes it difficult to consume the packages and make sense of changes, which is the point of doing releases 😄

ntwb · 2018-05-11T00:35:23Z

I don't feel like independent makes a lot of sense when individual packages from Gutenberg depend on each other.

This is what the cause of breaking SemVer would be due to, by setting this configuration option in Lerna every package in this repo would use the same version, but this doesn't have to mean semantic versioning is not used.

Rather, it would add SemVer to the Gutenberg releases, so if there is a breaking API change made later today, then the next release of Gutenberg would be 3.0.0 and 2.9.0 would be skipped.

The alternative with Lerna is to use the independent option which would allow Gutenberg 2.9.0 to be published including version 3.0.0 of @wordpress/date. Which is SemVer for the packages, but not Gutenberg itself.

tofumatt · 2018-05-11T01:40:01Z

Which is SemVer for the packages, but not Gutenberg itself.

That makes the most sense to me; Gutenberg's versioning is not SemVer, but we shouldn't condemn our packages to the same fate 😉

gziolo · 2018-05-11T07:31:28Z

The alternative with Lerna is to use the independent option which would allow Gutenberg 2.9.0 to be published including version 3.0.0 of @wordpress/date. Which is SemVer for the packages, but not Gutenberg itself.

Let's fly with independent and leave Gutenberg and WordPress following their own versioning schema. I thought it would be easier to follow Gutenberg versioning for some time, but apparently it would make things more confusing than I anticipated :)

ntwb · 2018-05-11T11:06:36Z

lerna.json

+	"packages": [
+		"packages/*"
+	],
+	"version": "independent"


gziolo · 2018-05-11T11:14:52Z

I'm merging this PR in the current shape with the intention to the follow-up work:

Nothing gets published to npm immediately.
I'm going to work on moving more modules to the same setup.
@youknowriad volunteered to help with integrating transpiled version for the distribution purpose (both es5 and esnext).
Let's discuss publishing strategy on Tuesday during CoreJS chat and all others steps.

gziolo added [Type] Technical Prototype Offers a technical exploration into an idea as an example of what's possible [Type] Build Tooling Issues or PRs related to build tooling labels May 9, 2018

gziolo self-assigned this May 9, 2018

gziolo requested review from tofumatt, youknowriad, ntwb, aduth and a team May 9, 2018 12:35

youknowriad reviewed May 9, 2018

View reviewed changes

tofumatt reviewed May 9, 2018

View reviewed changes

youknowriad reviewed May 9, 2018

View reviewed changes

ntwb reviewed May 9, 2018

View reviewed changes

gziolo requested a review from hypest May 9, 2018 13:54

youknowriad reviewed May 9, 2018

View reviewed changes

youknowriad approved these changes May 9, 2018

View reviewed changes

ntwb approved these changes May 9, 2018

View reviewed changes

gziolo force-pushed the update/move-date-to-packages branch from 19de1fa to 37a7223 Compare May 10, 2018 11:00

gziolo requested a review from pento May 10, 2018 11:00

gziolo and others added 9 commits May 11, 2018 12:17

Framework: Move date module to packages maintained by Lerna

365b70b

Move source code to src subfolder for date package

6b0ec7e

Date: Revert code that uses date modules instead of moment

312fa87

Build: Update Webpack config

5c05702

Build: Fix entry point for Gutenberg packages

fa750d9

Avoid global usage in the date module making it npm ready

7ec02f5

Chore: Do some cleanup for obsolete code

70af6a2

Docs: Copy releasing packages manual from packages repository

2be3596

Build: Use independent versioning for packages

8f022b8

gziolo force-pushed the update/move-date-to-packages branch from 37a7223 to 8f022b8 Compare May 11, 2018 10:52

Date: Add footer image to the Readme file

ecc985a

ntwb reviewed May 11, 2018

View reviewed changes

lerna.json

"packages": [

"packages/*"

],

"version": "independent"

Copy link

Member

ntwb May 11, 2018

Choose a reason for hiding this comment

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

👍

gziolo merged commit d70ec18 into master May 11, 2018

gziolo deleted the update/move-date-to-packages branch May 11, 2018 12:24

gziolo added this to the 2.9 milestone May 11, 2018

youknowriad mentioned this pull request May 11, 2018

Build: Add building/watching support to Gutenberg packages #6708

Merged

nerrad mentioned this pull request May 11, 2018

Make lerna a dependency rather than a devDependency #6718

Closed

This was referenced May 15, 2018

Packages: Move element to packages maintained by Lerna #6756

Merged

Packages: Move data module to the package maintained by Lerna #6828

Merged

This was referenced May 23, 2018

Deprecated: Introduce new module with depreaction util #6914

Merged

Blob: Extract new blob package out of utils module #6973

Merged

Shelob9 mentioned this pull request May 29, 2018

Packages: Publish all modules as independent npm packages #3955

Closed

19 tasks

gziolo mentioned this pull request Jun 18, 2018

Add momentjs object properties #7298

Closed

4 tasks

Build: Move date module to packages maintained by Lerna #6658

Build: Move date module to packages maintained by Lerna #6658

Conversation

gziolo commented May 9, 2018 • edited Loading

Description

Issues discovered

How has this been tested?

Types of changes

Checklist:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tofumatt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gziolo commented May 9, 2018

ntwb commented May 9, 2018

gziolo commented May 9, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ntwb commented May 9, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

youknowriad left a comment

Choose a reason for hiding this comment

aduth commented May 9, 2018

gziolo commented May 10, 2018

gziolo commented May 10, 2018

aduth commented May 10, 2018

tofumatt commented May 10, 2018

ntwb commented May 11, 2018

tofumatt commented May 11, 2018

gziolo commented May 11, 2018 • edited Loading

Choose a reason for hiding this comment

gziolo commented May 11, 2018

gziolo commented May 9, 2018 •

edited

Loading

gziolo commented May 9, 2018 •

edited

Loading

gziolo commented May 11, 2018 •

edited

Loading