Setup mono repository #3300

belcherj · 2016-02-14T22:46:04Z

This implementation assumes that all submodule dependencies are specified in the main package.json. When make run is run all submodules dependencies will be installed globally. Submodules will look up the directory tree to find the modules it requires and find them in the Calypso node_modules folder.

Script to check all submodule dependencies are specified in the Calypso package.json
Script to check for changes in folders with a package.json and ensure version is higher than the version in npm
Script to list modules that need to be published
Script to publish that module to npm

Steps to Make a Package Publishable:

Move code to a src folder
Change requires to reflect that move
Add a package.json file that contains all dependencies of the module
Provide a main property that defines the entrance for the module
Provide an initial version
Provide all the additional details needed in a package.json
Add README, CHANGELOG, LICENSE
Modify Makefile so it can run as part of whole and on its own
Provide an .npmignore file to keep src and other files from being published.
List all dependencies in the global package.json

Tug · 2016-02-15T08:03:55Z

client/lib/mixins/i18n/index.js

@@ -14,7 +14,7 @@ var debug = require( 'debug' )( 'calypso:i18n' ),
 var config = require( 'config' ),
 	numberFormatPHPJS = require( './number-format' ),
 	emitter = require( 'lib/mixins/emitter' ),
-	interpolateComponents = require( 'lib/interpolate-components' );
+	interpolateComponents = require( 'lib/interpolate-components/src/' );


Couldn't we use your new package.json "main" attribute to define the main entry point for this lib thus keeping this require clean?

After investigating this would require the module to be built on its own. The package.json already has a main property that is set to lib/index.js which is created once it is compiled. By providing th full path the component will be compiled as part of the parent projects build process. Do you you see an alternative?

Probably we can leave it for now like this and change it once we figure out how to deal with build steps.

By providing th full path the component will be compiled as part of the parent projects build process.

I'm not sure I follow that. By requiring directly into the component's inner-files, we're just requiring the pre-compiled /src/index.js file. Couldn't we just build the module as part of calypso's build process and then require it by name and then allow it to bring in the specified main file?

Or if we just want to require the original source file, could we change the location of interpolate-components inside the main package.json file to:

"interpolate-components": "client/lib/interpolate-components/src",

Requiring from a sub-directory in a published module will not find the package.json, the main reason to require it to install dependencies. I did not explain correctly how this PR works. Please see the main description which I have updated.

The package.json already has a main property that is set to lib/index.js which is created once it is compiled.

The package.json for interpolate-components in this PR has "main" set to src/index.js. Though I agree that we would want it set to the generated lib/index.js so the module works correctly for people requiring it through npm.

nb · 2016-02-16T16:57:54Z

Makefile

+	./bin/list-publishable-modules.js
+
+publish-module:
+	( cd $(path) && make publish )


Would this work in all shells?

Changing that line to make -C $(path) publish should do the trick.

Fixed in: https://github.com/belcherj/wp-calypso/commit/7e4530096e1173dbd13006fde69e4ad6f456832a

rralian · 2016-02-16T23:15:19Z

I think we should probably also add a .npmignore file inside the interpolate-components folder specifying that we should npmignore the src folder.

belcherj · 2016-02-18T03:17:31Z

@rralian I also included the following in the .npmignore: makefile and test folder. You can't run the makefile or tests without the src. Change in this commit: 3a209bc

nb · 2016-02-18T14:28:01Z

bin/check-package-version

+	process.exitCode = 0;
+}
+
+exec( 'npm view ' + packageJsonPath.name + ' version', function( error, stdout ) {


Can the name contain spaces or other shell special characters?

Good, call. I'll put in a patch.

Resolved in: 152aa0f

nb · 2016-02-25T08:43:09Z

I really like the approach.

The rest is in code comments, but one high-level question – how do you think we should deal with keeping module dependencies up to date with the global ones? For example, if we decide to upgrade to React 15, we will need to update all of the package.jsons of published packages.

Tug · 2016-02-25T09:41:36Z

bin/publish-modules

+}
+
+function publishModule( modulePath ) {
+	return exec( 'make -C ' + modulePath.slice( 0, -12 ) + ' publish' )


We could use the more standard npm publish command and run our tests with hooks such as prepublish. What do you think?

Unfortunately prepublish runs when you npm install a package from a local path: npm/npm#3059 That makes it difficult to use. To have that work all sub packages would have to have their dependencies installed.

deBhal · 2016-02-26T06:28:42Z

Nice work so far, this is a tricky problem :)

With this approach, where do our existing separate libraries fit in?

For example, I had to fix a problem in https://github.com/Automattic/xgettext-js, and it was pretty painful. You have to fix the module, hack the fix into calypso for testing, then go and commit and publish the fix in the module, then come back to calypso to pull in the latest version and make sure you haven't accidentally broken anything.

I'd love to be able to take advantage of our monorepo to pull in a module like this and smooth out that workflow. How do you see that fitting in with this approach?

belcherj · 2016-02-27T18:42:45Z

@deBhal:

You can include that module within the mono repo.
You can npm install from the local repository:

Build the npm module
Change the package.json to install from local
npm install
Run and test your module.

Option number two is basically the same as if the module was part of the mono-repo.

Also any script that you put into prepublish will run when a module is installed locally

belcherj · 2016-02-28T21:13:28Z

@nb When you update a dependency in the main package.json the pre-commit hook can check sub-module package.json files to see if those packages have been updated. This could either be a check or a requirement. The next step would be to automatically update the submodule dependency versions.

UPDATE: Looks like this functionality is already built in: bin/update-dependency I will test this script to ensure that it works and update here.

UPDATE: bin/update-dependency correctly updates dependencies in modules.

For without support for build step, but worked out versions and dependencies. Branch 'belcherj-try/mono-repo-1'

This reverts commit fa9ff88, reversing changes made to 477e0af. Broke build, probably missing shrinkwrap.

belcherj · 2016-03-04T01:13:44Z

This commit breaks the build because the Docker file copies over the package.json and installs without the rest of the repository so that the npm install process doesn't have to run on every build because of the docker cache: http://www.clock.co.uk/blog/a-guide-on-how-to-cache-npm-install-with-docker

To merge this PR caching in Docker for npm installed would have to be invalidated each time.

UPDATE: This has been resolved by not installing submodules. Instead list all dependencies in Calypso package.json.

…ckage.json

belcherj · 2016-05-10T14:56:58Z

This PR was a failed approach to independently publishing NPM modules from within the main repository.

bluefuton added Framework [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Feb 14, 2016

Tug reviewed Feb 15, 2016
View reviewed changes

lancewillett added the OSS Citizen label Feb 15, 2016

nb reviewed Feb 16, 2016
View reviewed changes

nb reviewed Feb 18, 2016
View reviewed changes

nb mentioned this pull request Feb 25, 2016

Framework: try monorepo approach for publishing independent modules #2396

Closed

nb added [Status] Needs Author Reply and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Feb 25, 2016

Tug reviewed Feb 25, 2016
View reviewed changes

aduth added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. and removed [Status] Needs Author Reply labels Feb 29, 2016

nb added a commit that referenced this pull request Mar 1, 2016

Merge PR #3300: Setup mono repository

fa9ff88

For without support for build step, but worked out versions and dependencies. Branch 'belcherj-try/mono-repo-1'

nb added a commit that referenced this pull request Mar 1, 2016

Revert "Merge PR #3300: Setup mono repository"

cc2cd91

This reverts commit fa9ff88, reversing changes made to 477e0af. Broke build, probably missing shrinkwrap.

ehg mentioned this pull request Mar 1, 2016

Build: Make sure npm has an updated shrinkwrap to work with #3687

Closed

belcherj closed this Mar 2, 2016

belcherj reopened this Mar 2, 2016

belcherj removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Mar 3, 2016

lancewillett added the [Status] In Progress label Mar 4, 2016

belcherj added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. and removed [Status] In Progress labels Mar 8, 2016

belcherj added 5 commits March 10, 2016 19:46

Setup Interpolate Components to be publishable

e4c4574

Add script to publish modules

50ed65f

Add precommit hook to check for packages needing update

8f7313b

Add checks to ensure submodule dependencies are registered in main pa…

25ca3c5

…ckage.json

Update shrinkwrap

1405831

belcherj added [Status] In Progress and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Mar 16, 2016

belcherj closed this May 10, 2016

matticbot removed the [Status] In Progress label May 10, 2016

lancewillett mentioned this pull request Jun 2, 2016

Framework: Update/use open interpolate components #2247

Closed

davewhitley mentioned this pull request Sep 23, 2016

Localization Automattic/simplenote-electron#16

Closed

belcherj mentioned this pull request Sep 21, 2018

Build: A basic monorepo setup #27356

Merged

3 tasks

This was referenced Feb 20, 2022

[Snyk] Security upgrade prismjs from 1.15.0 to 1.27.0 ajesse11x/wp-calypso#73

Open

[Snyk] Security upgrade prismjs from 1.16.0 to 1.27.0 awaisab172/wp-calypso#110

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Setup mono repository #3300

Setup mono repository #3300

belcherj commented Feb 14, 2016

Tug Feb 15, 2016

belcherj Feb 15, 2016

belcherj Feb 15, 2016

nb Feb 16, 2016

rralian Feb 16, 2016

belcherj Feb 16, 2016

rralian Feb 16, 2016

nb Feb 16, 2016

klimeryk Feb 16, 2016

belcherj Feb 16, 2016

rralian commented Feb 16, 2016

belcherj commented Feb 18, 2016

nb Feb 18, 2016

belcherj Feb 18, 2016

belcherj Feb 19, 2016

nb commented Feb 25, 2016

Tug Feb 25, 2016

belcherj Feb 27, 2016

deBhal commented Feb 26, 2016

belcherj commented Feb 27, 2016

belcherj commented Feb 28, 2016

belcherj commented Mar 4, 2016

belcherj commented May 10, 2016

Setup mono repository #3300

Setup mono repository #3300

Conversation

belcherj commented Feb 14, 2016

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

rralian commented Feb 16, 2016

belcherj commented Feb 18, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nb commented Feb 25, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

deBhal commented Feb 26, 2016

belcherj commented Feb 27, 2016

belcherj commented Feb 28, 2016

belcherj commented Mar 4, 2016

belcherj commented May 10, 2016