Skip to content

Conversation

@michaelsbradleyjr
Copy link
Contributor

@michaelsbradleyjr michaelsbradleyjr commented Jan 31, 2019

TL;DR

yarn install in a fresh clone of the repo.

yarn reboot when switching branches.

When pulling in these changes, there may be untracked files at the root in all/some of:

.embark/
.nyc_output/
coverage/
dist/
embark-ui/
test_apps/

They can be safely deleted since those paths are no longer in use at the root.

Many of the scripts in the top-level package.json support Lerna's filter options. For example:

yarn build --scope embark build only packages/embark.

yarn build --ignore embark-ui build everything except packages/embark-ui.

Scoping scripts will be more useful when there are more packages in the monorepo and, for example, yarn start doesn't need to be invoked for all of them while working on just a few of them simultaneously, e.g embark and embarkjs.

It's also possible to cd into a particular package and run its scripts directly:

cd packages/embark && yarn watch

Hot Topics & Questions

What should be done about the README for packages/embark? Should the top-level README be duplicated in that package?

Lerna is setup to use Fixed/Locked mode, and accordingly packages/embark-ui is set to 4.0.0-beta.0. The same will be true when adding embarkjs, swarm-api, etc. to the monorepo. Is this acceptable or do we want to use Independent mode?

Scripts

If a package doesn't have a matching script, lerna run skips it automatically. For example, packages/embark-ui doesn't have a typecheck script.

yarn build

Runs babel, webpack, etc. according to a package's build script.

yarn build:no-ui is a shortcut for yarn build --ignore embark-ui.

yarn ci

Runs a series of scripts relevant in a CI context according to a package's ci script. For packages/embark that's lint typecheck build test package.

Also runs the ci script of the embedded test_dapps monorepo.

yarn clean

Runs rimraf, etc. according to a package's clean script.

yarn globalize

Makes the development embark available on the global PATH, either via symlink (Linux, macOS) or a shim script (Windows).

yarn lint

Runs eslint, etc. according to a package's lint script.

yarn package

Invokes npm pack according to a package's package script.

yarn qa

Very similar to ci, runs a series of scripts according to a package's qa script. The big difference between ci and qa is that at the top-level qa first kicks off reboot:full.

There is a preqa script (invoked automatically), which is a bit of a wart. It makes sure that embark reset can be run successfully in packages/embark/templates/* when the reboot script invokes the reset script.

The qa script is invoked by yarn release before the latter proceeds to invoke lerna publish.

yarn reboot

Invokes the reset script and then does yarn install.

The reboot:full variant invokes reset:full and then does yarn install.

yarn release

Works in concert with lerna publish, which will prompt to verify the version before proceeding. Use n to cancel instead of ctrl-c as lerna publish has been seen to occasionally misbehave when not exited cleanly (e.g. creating a tag when it shouldn't have).

yarn release [bump] [--options]
  • [bump] see publish positionals and version positionals; an exact version can also be specified.
  • --preid prerelease identifier, e.g. beta; when doing a prerelease bump will default to whatever identifier is currently in use.
  • --dist-tag registry distribution tag, defaults to latest.
  • --message commit message format, defaults to chore(release): %v.
  • --sign indicates that the git commit and tag should be signed; not signed by default.
  • --release-branch default is master; must match the current branch.
  • --git-remote default is origin.
  • --registry default is https://registry.npmjs.org/ per the top-level lerna.json.

To release 4.0.0-beta.1 as embark@next (assuming version is currently at 4.0.0-beta.0) could do:

yarn release prerelease --dist-tag next

For test releases (there is no longer a --dry-run option) verdaccio and a filesystem git remote can be used.

Condensend instructions:

mkdir -p ~/temp/clones && cd ~/temp/clones
git clone git@github.com:embark-framework/embark.git
cd ~/repos/embark
git remote add FAKEembark ~/temp/clones/embark

in another terminal:

npm i -g verdaccio && verdaccio

in the first terminal:

yarn release --git-remote FAKEembark --registry http://localhost:4873/

yarn reset

Invokes cleaning and resetting steps according to a package's reset script. The big difference between clean and reset is that reset is intended to delete a package's node_modules.

The reset:full variant deletes the monorepo's top-level node_modules at the end. That shouldn't be necessary too often, e.g. in day-to-day work when switching branches, which is why there is reboot / reset vs. reboot:full / reset:full.

Errors may be seen related to invocation of embark reset if embark is not built, but reset will still complete successfully.

yarn start

Runs babel, webpack, tsc, etc. (in parallel, in watch mode) according to a package's start script.

yarn test

Run mocha, etc. according to a package's test script.

The test:full variant runs a series of scripts: lint typecheck test test_dapps.

yarn test_dapps

Runs the test script of the embedded test_dapps monorepo.

The test_dapps:ci and test_dapps:qa variants run the ci and qa scripts of the embedded test_dapps monorepo, respectively.

yarn typecheck

Runs tsc, etc. according to a package's typecheck script.

Notes

npx is used in some of the top-level and package scripts to ensure the scripts can run even if node_modules is missing.

"nohoist" specifies a couple of embark packages because restrictPath is interfering with access to modules that are located in a higher-up node_modules.

All dependencies in packages/embark-ui have been made devDependencies since its production build is self-contained.

packages/embark's existing CHANGELOG's formatting has been slightly adjusted to match the formatting that Lerna will use going forward (entries in the log haven't been modified).

Lerna will generate a CHANGELOG at the top-level and in each package. Since we're transitioning to a monorepo, things may look a little wonky with respect to old entries in packages/embark/CHANGELOG.md and going forward we need to consider how scoping our commits corresponds to member-packages of the monorepo.

In packages/embark, test invokes scripts/test, which starts a child process wherein process.env.DAPP_PATH is a temporary path that has all of packages/embark/dist/test copied into it, so that paths to test helpers/fixtures don't need to be prefixed with dist/test/ and so that a .embark directory doesn't get written into packages/embark.

The "engines" specified in top-level and packages' package.json reflect a node and npm pair that match (a source of confusion in the past). The pair was chosen according to the first post v5 npm that's bundled with node. A "runtime" key/object has been introduced in packages/embark/package.json which is used as the basis for specifying the minimum version of node that can be used to run embark, and that's what is checked by bin/embark.

Some changes have been introduced, e.g. in lib/core/config and lib/utils/solidity/remapImports so that it's not implicitly assumed that process.env.DAPP_PATH / fs.dappPath() are the same as process.cwd(). There are probably several++ places where that assumption is still in effect, and we should work to identify and correct them.

embark reset now deletes embarkArtifacts/ within a dapp root, and embarkArtifacts/ is git-ignored.

lib/core/env adds all node_modules relative to process.env.EMBARK_PATH to NODE_PATH so that embark's modules can be resolved as expected whether embark's node_modules have been deduped or are installed in npm's flat "global style".

checkDependencies has been inlined (see lib/utils/checkDependencies) and slightly modified to support dependencies that have been hoisted into a higher-up node_modules, e.g. as part of a yarn workspace. eslint has been disabled for that script to avoid more involved changes to it.

test_apps is not in packages/embark; rather, there is test_dapps at the top-level of the monorepo. test_dapps is an embedded monorepo, and its ci / qa scripts npm install embark from freshly built tarballs of the packages in the outer monorepo and then use that installation to run embark test in the dapps. This should allow us to rapidly detect breakage related to auto-bumps in transitive dependencies.

@alaibe
Copy link
Contributor

alaibe commented Jan 31, 2019

The PR looks good to me I just have one question.
Why did you make test_apps like a secondary mono repo
and have packages inside test_apps/packages instead of putting the test_apps directly as a subfolder of test_app.

Regarding the templates do you think it would also be possible to extract them from embark and put them inside a templates directory?
That would I guess be a way to regroup all of them

@michaelsbradleyjr michaelsbradleyjr force-pushed the build/lerna branch 2 times, most recently from a6e401c to 8809b51 Compare February 1, 2019 02:41
@michaelsbradleyjr
Copy link
Contributor Author

michaelsbradleyjr commented Feb 1, 2019

Regarding the templates do you think it would also be possible to extract them from embark and put them inside a templates directory?

Yes, definitely possible to extract the templates, and I think we should. I tried making them siblings of embark in packages/ but ran into a couple of difficulties (which I think I understand better now) so left them as-is. Also, making them dependencies would require some changes to the template generator and I felt that would be best left to another PR.

Why did you make test_apps like a secondary mono repo and have packages inside test_apps/packages instead of putting the test_apps directly as a subfolder of test_app?

The monorepo pattern is ideal when you have a set of packages grouped together and you want to do the same steps for each of them, e.g. invoke package.json test to launch embark test. At the same time, the test dapps seem a bit different in scope than packages that do/will make up embark, so I carved them off into a separate monorepo. Also, it's easier in that setup to have a pre-test routine (in ci/qa context) specific to the dapps (i.e. as a group, before individual testing) that installs an embark from fresh tarballs (using npm install, so no lock file) built in the top-level monorepo and then tests the dapps using that embark. That way, we can quickly detect whether we're up against any transitive dependency bumps that will cause problems for users installing from the registry.

Ideally, I think test_dapps and dapp-bin can be unified in a monorepo that's separate from the embark monorepo. We can use CI triggers so that when there are pushes to embark-framework/embark it causes embark-framework/dapp-bin (on travis, appveyor) to clone the relevant embark branch, build it and test all of the dapps against that build. We can use the trick of giving a git url/shortcut to npm install, or use git submodules, so that we don't have to copy the sources of 3rd party dapps into dapp-bin, just keep them in a list; and if each embark template is a standalone package we can use the same technique for them. For day-to-day testing with and development of dapps in dapp-bin, one could cd to the members of dapp-bin and used globalized embark test, embark run, etc.

Copy link
Collaborator

@emizzle emizzle left a comment

Choose a reason for hiding this comment

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

Feedback

Firstly, hats off to some great work! I know this took an insane amount of effort and head banging, so congrats on getting it across the line! The PR looks great. I have been playing around with it since I based the remap import updates off of this branch, and I have to say that it is definitely easier to get around the different yarn/npm commands than the current master. I believe we will get used to these changes quite quickly.

The test_dapps is a great idea for detecting changes. I've noticed that trying to embark run the test dapps inside of their location in the monorepo triggers the check on the dapp regarding the dapp path length, which is known to cause errors starting geth. Obviously the way around this is to copy out the test apps to a different location in the filesystem. Because these test dapps are now in their own mono repo, would it be possible to quickly npx install them in to a folder and run them from there?

Regarding nohoist, I noticed this is only for the two packages added for use in the embark unit tests (embark/embark-test-contract-0 and embark/embark-test-contract-1). I believe we could actually move these two packages to devDependencies of packages/embark. Would that move alter the need for nohoist?

I used yarn globalize and have two points of feedback:

  1. Firstly, I had an existing symlink /usr/local/bin/embark so it errored. Not a big deal, I deleted the original and re-ran yarn globalize.
  2. After running yarn globalize, the symlink was created, but running which embark still shows /Users/emizzle/.nvm/versions/node/v10.5.0/bin/embark, as it is in my PATH first. I'm assuming NVM does this for me. I guess there isn't much of a way around this, and maybe this is overkill, but there could be a check after yarn globalize comparing the output of which embark to it's expected value and warning the user if they don't match.

Hot topics

  1. What should be done about the README for packages/embark? Should the top-level README be duplicated in that package?
  • Could we maybe just have a link to the top-level README?
  1. Lerna is setup to use Fixed/Locked mode, and accordingly packages/embark-ui is set to 4.0.0-beta.0. The same will be true when adding embarkjs, swarm-api, etc. to the monorepo. Is this acceptable or do we want to use Independent mode?
  • Hmm that is a good question. Personally, I would say that it wouldn't make sense to bump all modules/packages every time embark is updated as the majority of the modules/package would actually not have changed, so personally I would prefer Independent Mode.

@iurimatias
Copy link
Collaborator

I also think we should use independent mode

@michaelsbradleyjr
Copy link
Contributor Author

michaelsbradleyjr commented Feb 1, 2019

The test_dapps is a great idea for detecting changes. I've noticed that trying to embark run the test dapps inside of their location in the monorepo triggers the check on the dapp regarding the dapp path length, which is known to cause errors starting geth.

Maybe we can remove that check on the dapp path length owing to the changes in #1214. It probably should have been removed in that PR to begin with (something I overlooked).

Independent of that concern, in a qa / ci context, I think it would be good to copy the whole test_dapps monorepo outside of the top-level monorepo's directory structure before running its scripts. Likewise, the "packaged" embark that test_dapps sets up should be installed in a directory outside of the relocated (copied) test_dapps, also outside of the top-level monorepo's dir structure. Those changes together should greatly reduce the chance that a module is inadvertently being resolved (by a dapp) in a way that won't apply in a production context. I think, though, the PR can land as-is and those improvements can be made in another PR. Thoughts?

I believe we could actually move these two packages to devDependencies of packages/embark. Would that move alter the need for nohoist?

Unfortunately, it would not remove the need; note that they are already devDeps of packages/embark.

I used yarn globalize and have two points of feedback...

I will look into those issues shortly.

Could we maybe just have a link to the top-level README?

@iurimatias any thoughts?

Personally, I would say that it wouldn't make sense to bump all modules/packages every time embark is updated...

That's not the behavior of fixed mode, though granted that's what the name seems to imply. I am going to write-up a reply to Iuri's comment re: the mode choice and attempt to explain why it may be better to stick with fixed mode, for now.

Copy link
Collaborator

@jrainville jrainville left a comment

Choose a reason for hiding this comment

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

Very nice work. Just a couple of small questions

"engines": {
"node": ">=8.11.3",
"npm": ">666",
"node": ">=8.12.0",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why bump the node version? Is 8.11 no good now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The "engines" specified in top-level and packages' package.json reflect a node and npm pair that match (a source of confusion in the past). The pair was chosen according to the first post v5 npm that's bundled with node. A "runtime" key/object has been introduced in packages/embark/package.json which is used as the basis for specifying the minimum version of node that can be used to run embark, and that's what is checked by bin/embark.

By "that match", I meant they are bundled together, i.e. if you install node v8.12.0 you get npm v6.4.1. Any earlier version of node includes npm v5.x.

For the runtime, node v8.11.3 is still our minimum supported version.

"node": ">=8.11.3",
"npm": ">666",
"node": ">=8.12.0",
"npm": ">=6.4.1",
Copy link
Collaborator

Choose a reason for hiding this comment

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

So, we can install with npm now? Or is yarn still recommended? (I'll still use yarn, just curious)

Copy link
Contributor Author

@michaelsbradleyjr michaelsbradleyjr Feb 1, 2019

Choose a reason for hiding this comment

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

It's important to use yarn at the top-level since "workspaces" is not supported by npm.

npm is still used in various ways in the development setup, and since v5 is pretty buggy, I pegged it to the first v6.x npm that got bundled with a node release, i.e. node v8.12.0. Given when/how npm enforces the engines check with respect to itself, it's more of a "signal to humans" than an automatic protection. In any case, it only applies to developers working on embark itself.

Note that owing to the monorepo setup, it becomes impossible to install embark directly from github since hosted-git-info (used internally by npm) doesn't have a mechanism for identifying a specific package within the directory structure of a git repository. Yarn doesn't offer support for that either. It's a trade-off: installing from a repo is a nice capability, sad to lose it, but the benefits of the monorepo will hopefully outweigh the loss.

Copy link
Contributor Author

@michaelsbradleyjr michaelsbradleyjr Feb 1, 2019

Choose a reason for hiding this comment

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

Maybe we can adapt #1263 so that if a developer tries to npm install a clone of the repo (npm install embark from registry is fine), then it will fail with an error message saying "use yarn". Thoughts?

Copy link
Contributor Author

@michaelsbradleyjr michaelsbradleyjr Feb 1, 2019

Choose a reason for hiding this comment

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

I looked into the technique of #1263 and found it won't work. There doesn't seem to be a way, via environment variables available to the preinstall script, to distinguish npm install in the repo vs. npm install [pkg], where [pkg] is a tarball path or identifier on the registry or was listed as a dev/Dep in a package.json.

Since we want users to be able to install embark from the registry using npm, I think it's a no-go. We'll just need to educate developers who want to work on embark or run a development build of embark that they need to yarn install in the top-level of the cloned repo instead of doing npm install.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, personally I think that is probably the right approach. Since npm installing embark from the registry is OK, and only developers wishing to contribute to embark are required to use yarn, adding some information where necessary (on the README and website) would be sufficient.

fs.removeSync('.embark/');
fs.removeSync('node_modules/.cache');
fs.removeSync('dist/');
fs.removeSync('embarkArtifacts/');
Copy link
Collaborator

Choose a reason for hiding this comment

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

This can be configured by the user. Maybe we should check what's in embark.json first. Same for dist/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. I guess embark reset needs some attention, but do you think it's okay to address that in a future PR?

@michaelsbradleyjr
Copy link
Contributor Author

michaelsbradleyjr commented Feb 1, 2019

@iurimatias @emizzle regarding fixed vs. independent mode:

I think, based on the gitter chat log, that one point of confusion has already been cleared up: both fixed and independent modes only bump a version and publish a package if that package has changed.

By using caret ranges for our monorepo packages, if we're committed to adhering to semver for our own stuff, we get some nice flexibility for ourselves and also our users, e.g. embark@4.1.0 could automatically pick up embark-ui@4.1.1 or even embark-ui@4.2.0 (anything less than 5.0.0, actually).

Fixed

I think the advantage of fixed mode is that we don't have to make manual semver calculations when we hit a major release. For example: if we introduce a breaking change in embarkjs and bump to 5.0.0 then everything else will bump to 5.0.0 also.

That the sibling packages all bump isn't strictly required by semver, e.g. embark@4.2.0 could be supplied by us with a capability to work with either v4 or v5 of embarkjs, and a semver range for embarkjs could be included in embark's package.json to that effect ("embarkjs": "^4.1.3 || ^5.0.0"). On the other hand, for humans working with a set of related packages, e.g. embark-*, it can be less confusing if all of the major version numbers bump in tandem.

Independent

I think independent mode probably becomes useful when you have loosely related groups of packages all in one monorepo and it's undesirable that a major version bump to one package in one group will induce a major bump in all of the packages across all groups. The trade-off is that independent mode will require less-automated semver calculations (and more of them) when doing releases, and I suppose that could be an error-prone exercise in a large monorepo.

I don't think our monorepo will have that structure in the near term, so for now I think it's simpler for us to stick with fixed mode, and reconsider if we find it limiting.

@michaelsbradleyjr
Copy link
Contributor Author

@emizzle

Firstly, I had an existing symlink /usr/local/bin/embark so it errored. Not a big deal, I deleted the original and re-ran yarn globalize.

scripts/globalize.js attempts to delete (macOS, Linux) or overwrite (Windows) an existing embark symlink / shim-script, so I'm guessing there must have been a permissions problem. I am unable to reproduce the problem on my machines unless I create a permissions mismatch.

After running yarn globalize, the symlink was created, but running which embark still shows /Users/emizzle/.nvm/versions/node/v10.5.0/bin/embark, as it is in my PATH first. I'm assuming NVM does this for me. I guess there isn't much of a way around this, and maybe this is overkill, but there could be a check after yarn globalize comparing the output of which embark to it's expected value and warning the user if they don't match.

Can you check the output of npm --global bin? That's the directory into which yarn globalize (scripts/globalize.js) will attempt to write.

@michaelsbradleyjr michaelsbradleyjr force-pushed the build/lerna branch 2 times, most recently from ca52a2f to d9b816f Compare February 1, 2019 17:59
@emizzle
Copy link
Collaborator

emizzle commented Feb 3, 2019

independent of that concern, in a qa / ci context, I think it would be good to copy the whole test_dapps monorepo outside of the top-level monorepo's directory structure before running its scripts. Likewise, the "packaged" embark that test_dapps sets up should be installed in a directory outside of the relocated (copied) test_dapps, also outside of the top-level monorepo's dir structure. Those changes together should greatly reduce the chance that a module is inadvertently being resolved (by a dapp) in a way that won't apply in a production context. I think, though, the PR can land as-is and those improvements can be made in another PR.

Agreed.

scripts/globalize.js attempts to delete (macOS, Linux) or overwrite (Windows) an existing embark symlink / shim-script, so I'm guessing there must have been a permissions problem. I am unable to reproduce the problem on my machines unless I create a permissions mismatch.

A permissions issue does ring a bell and that was most likely the cause.

yarn globalize issue

Can you check the output of npm --global bin? That's the directory into which yarn globalize (scripts/globalize.js) will attempt to write.

The output of npm --global bin is /Users/emizzle/.nvm/versions/node/v10.5.0/bin. Yet it seems like yarn globalize tried to install in /usr/local/bin, hence the permissions error. The code for this is very straightforward, and I'm not sure how it could try to use /usr/local/bin instead of /Users/emizzle/.nvm/versions/node/v10.5.0/bin. Additionally, the symlink for /Users/emizzle/.nvm/versions/node/v10.5.0/bin/embark points to ../lib/node_modules/embark/bin/embark. And running embark version still gives me 3.2.7.

After doing some further testing, running node scripts/globalize.js directly, installs the correct symlink. But doing yarn globalize does not. Repro:

Working

  1. Unlink the embark symlink in /Users/emizzle/.nvm/versions/node/v10.5.0/bin
  2. Run node scripts/globalize.js
  3. Symlink appears correctly /Users/emizzle/.nvm/versions/node/v10.5.0/bin/embark -> /Users/emizzle/Code/__Github/embk-fw/embark/packages/embark/bin/embark.

Not working

  1. Unlink the embark symlink in /Users/emizzle/.nvm/versions/node/v10.5.0/bin
  2. Run yarn globalize.
  3. Symlink does not appear in /Users/emizzle/.nvm/versions/node/v10.5.0/bin

So it seems that the symlink I had previously (pointing to ../lib/node_modules/embark/bin/embark) most likely existing before ever running yarn globalize, probably from my last npm install -g embark.

Fixed vs independent modes

After the discussion, it does make sense to keep this as fixed mode to prevent manual semver bumping, and to bump all packages with major version updates.

@0x-r4bbit
Copy link
Contributor

0x-r4bbit commented Feb 4, 2019

I know I'm a little late to the party but here's my feedback considering all comments made:

README

What should be done about the README for packages/embark? Should the top-level README be duplicated in that package?

So either way, I think the top level readme should make the following points very clear:

  • What's this project about?
  • Why do I care? (quickly tease what it's good at)
  • How can I use it/get started (short installation instructions, then link to the docs ASAP)
  • Where do I find things in this repo? (short overview what packages are relevant e.g. cli vs cockpit)
  • How can I get involved? (Link to relevant places, contributing guidelines,docs etc)

I guess the idea would be to keep the README short and concise and every project should have its own README (CLI, Cockpit, soon Jetpack and Embark SDK(?))

Fixed vs Independent mode

Lerna is setup to use Fixed/Locked mode, and accordingly packages/embark-ui is set to 4.0.0-beta.0. The same will be true when adding embarkjs, swarm-api, etc. to the monorepo. Is this acceptable or do we want to use Independent mode?

Some random thoughts on this:

  • I think it's good to have the same versions for packages that belong to a projects (notice we have different projects in this repo: CLI, Cockpit, soon Jetpack, Embark as a library). Once we get to a point where people actually install individual modules for their needs, it'd definitely be good to have them in the same version. E.g. @embark/core, @embark/blockchain, @embark/ipfs <- these could all have the same version. This is the same way that for example Angular goes: @angular/core, @angular/platform-browser, @angular/router etc, all share the same version.
  • However, other project should maintain their own version. For example, I think Cockpit's version is only coupled to Embark's version in the sense that it has Embark as a dependency. It may depend on @embark/core etc. but changes in the deps version numbers should only affect Cockpit's version when there's a breaking change in the underlying dependency.
  • Same goes for EmbarkJS (soon Jetpack). it's going to be fully standalone and increasing its version number just for the sake of increasing it will also confuse people.

I assume that it's not possible to have a "mixed" mode where for some packages we have fixed mode and for others independent mode. But considering the thoughts above, I have the feeling independent mode is what makes more sense.

yarn globalize

  • I had it used once and it worked fine, but then there was something weird going on when "uninstalling"/"unlinking" the package. I can't fully recall what was happening, but it took a bit of effort to get rid off the global link again so I can use a normal installation.
  • I personally will probably just not use this command at all, as I like to keep control of things

@michaelsbradleyjr
Copy link
Contributor Author

@emizzle it should work correctly if you do npm run globalize. I think I've figured out the problem:

If I brew install yarn (which installs node in /usr/local/bin/) instead of npm i -g yarn, then I get the behavior where yarn globalize is writing to /usr/local/bin/ vs. /Users/michael/.nvm/versions/node/v10.15.1/bin, even though nvm's node is the one being used (higher precedence on global PATH)

That shouldn't happen because of the scripts-prepend-node-path setting in the monorepo's .yarnrc. I'm fairly certain that's a bug (regression) in yarn and I've informed the yarn maintainers. I actually came across this weeks ago, reported the regression, and switched to installing yarn via npm in my fresh node environments. But then I forgot about my discovery, until now.

Long story short: you can do npm run globalize or node scripts/globalize, and/or (since you're using nvm) brew uninstall yarn node && npm i -g yarn. If/when the yarn bug is fixed, yarn globalize will respect the --global bin of nvm's node regardless of how yarn was installed.

@michaelsbradleyjr
Copy link
Contributor Author

michaelsbradleyjr commented Feb 4, 2019

@PascalPrecht

I assume that it's not possible to have a "mixed" mode where for some packages we have fixed mode and for others independent mode. But considering the thoughts above, I have the feeling independent mode is what makes more sense.

Unfortunately, there's no "mixed" mode. For independent mode, I think I'll need to slightly adjust the release script, as I'm not sure if/how the [bump] positional argument comes into play in that mode.

I personally will probably just not use this command at all, as I like to keep control of things

For perspective: my goal with globalize was/is to increase the level of developer control, since in the past npm link seemed prone to weird behavior — could end up with broken installations when un/re-installing global embark (npm link always runs npm install automatically and would sometimes do really weird stuff like top-level global installs of transitive deps of embark during un/re-install). I personally stopped npm link'ing last Fall and manually put a symlink in npm --global bin to ~/repos/embark/bin/embark to take control of the situation. The globalize script simply automates that step, and takes care of Windows devs as well with a shim script.

In any case, after additional experiments this morning, I've realized having our own globalize script may be unnecessary. If top-level of the monorepo is ~/repos/embark then you can do cd ~/repos/embark/packages/embark && yarn link. Just like globalize, yarn link writes a symlink pointing (effectively) to ~/repos/embark/packages/embark/bin/embark in yarn global bin, which in an nvm context on Linux/macOS is the same as npm --global bin, e.g. /Users/michael/.nvm/versions/node/v10.15.1/bin. On Windows, for yarn link to make embark globally available, it's important to have yarn global bin in the global PATH, e.g. C:\Users\micha\AppData\Local\Yarn\bin. Happily, yarn link doesn't trigger install, it just sets up the links/shims.

For everyone working on embark, once we merge this PR I highly recommend no longer using npm link, i.e. for packages/embark in the monorepo, as that will conflict with the node_modules arrangement of the yarn-workspace and things may break in weird ways. Use yarn link instead.

I had it used once and it worked fine, but then there was something weird going on when "uninstalling"/"unlinking" the package. I can't fully recall what was happening, but it took a bit of effort to get rid off the global link again so I can use a normal installation.

My guess is you ran afoul of a bug in yarn, which I discovered and then forgot about when I started working around it, see my comment to Eric above. My apologies for not giving a heads-up earlier re: that bug; hopefully the yarn maintainers will fix it.

TL;DR
=====

`yarn install` in a fresh clone of the repo.

`yarn reboot` when switching branches.

When pulling in these changes, there may be untracked files at the root in
all/some of:

```
.embark/
.nyc_output/
coverage/
dist/
embark-ui/
test_apps/
```

They can be safely deleted since those paths are no longer in use at the root.

Many of the scripts in the top-level `package.json` support Lerna's [filter
options]. For example:

`yarn build --scope embark` build only `packages/embark`.

`yarn build --ignore embark-ui` build everything except `packages/embark-ui`.

Scoping scripts will be more useful when there are more packages in the
monorepo and, for example, `yarn start` doesn't need to be invoked for all of
them while working on just a few of them simultaneously, e.g `embark` and
`embarkjs`.

It's also possible to `cd` into a particular package and run its scripts
directly:

```
cd packages/embark && yarn watch
```

Hot Topics & Questions
======================

What should be done about the [README][embark-readme] for `packages/embark`?
Should the top-level README be duplicated in that package?

Lerna is setup to use [Fixed/Locked mode][fixed-locked], and accordingly
`packages/embark-ui` is set to `4.0.0-beta.0`. The same will be true when
adding embarkjs, swarm-api, etc. to the monorepo. Is this acceptable or do we
want to use [Independent mode][independent]?

Scripts
=======

If a package doesn't have a matching script, `lerna run` skips it
automatically. For example, `packages/embark-ui` doesn't have a `typecheck`
script.

`yarn build`
------------

Runs babel, webpack, etc. according to a package's `build` script.

`yarn build:no-ui` is a shortcut for `yarn build --ignore embark-ui`.

`yarn ci`
---------

Runs a series of scripts relevant in a CI context according to a package's `ci`
script. For `packages/embark` that's `lint typecheck build test package`.

Also runs the `ci` script of the embedded `test_dapps` monorepo.

`yarn clean`
------------

Runs rimraf, etc. according to a package's `clean` script.

`yarn globalize`
----------------

Makes the development embark available on the global PATH, either via
symlink (Linux, macOS) or a shim script (Windows).

`yarn lint`
-----------

Runs eslint, etc. according to a package's `lint` script.

`yarn package`
--------------

Invokes `npm pack` according to a package's `package` script.

`yarn qa`
---------

Very similar to `ci`, runs a series of scripts according to a package's `qa`
script. The big difference between `ci` and `qa` is that at the top-level `qa`
first kicks off `reboot:full`.

There is a `preqa` script ([invoked automatically][npm-scripts]), which is a
bit of a wart. It makes sure that `embark reset` can be run successfully in
`packages/embark/templates/*` when the `reboot` script invokes the `reset`
script.

The `qa` script is invoked by `yarn release` before the latter proceeds to
invoke `lerna publish`.

`yarn reboot`
-------------

Invokes the `reset` script and then does `yarn install`.

The `reboot:full` variant invokes `reset:full` and then does `yarn install`.

`yarn release`
--------------

Works in concert with [lerna publish], which will prompt to verify the version
before proceeding. Use `n` to cancel instead of `ctrl-c` as `lerna publish` has
been seen to occasionally misbehave when not exited cleanly (e.g. creating a
tag when it shouldn't have).

```
yarn release [bump] [--options]
```

* `[bump]` see [`publish` positionals][pub-pos] and [`version`
  positionals][ver-pos]; an exact version can also be specified.
* `--preid` prerelease identifier, e.g. `beta`; when doing a prerelease bump
  will default to whatever identifier is currently in use.
* `--dist-tag` registry distribution tag, defaults to `latest`.
* `--message` commit message format, defaults to `chore(release): %v`.
* `--sign` indicates that the git commit and tag should be signed; not signed
  by default.
* `--release-branch` default is `master`; must match the current branch.
* `--git-remote` default is `origin`.
* `--registry` default is `https://registry.npmjs.org/` per the top-level
  [`lerna.json`][lerna-json].

To release `4.0.0-beta.1` as `embark@next` (assuming version is currently at
`4.0.0-beta.0`) could do:

```
yarn release prerelease --dist-tag next
```

For *test releases* (there is no longer a `--dry-run` option) [verdaccio] and a
filesystem git remote can be used.

Condensend instructions:

```
mkdir -p ~/temp/clones && cd ~/temp/clones
git clone git@github.com:embark-framework/embark.git
cd ~/repos/embark
git remote add FAKEembark ~/temp/clones/embark
```
in another terminal:
```
npm i -g verdaccio && verdaccio
```
in the first terminal:
```
yarn release --git-remote FAKEembark --registry http://localhost:4873/
```

`yarn reset`
------------

Invokes cleaning and resetting steps according to a package's `reset`
script. The big difference between `clean` and `reset` is that `reset` is
intended to delete a package's `node_modules`.

The `reset:full` variant deletes the monorepo's top-level `node_modules` at the
end. That shouldn't be necessary too often, e.g. in day-to-day work when
switching branches, which is why there is `reboot` / `reset` vs. `reboot:full`
/ `reset:full`.

Errors may be seen related to invocation of `embark reset` if embark is not
built, but `reset` will still complete successfully.

`yarn start`
------------

Runs babel, webpack, tsc, etc. (in parallel, in watch mode) according to a
package's `start` script.

`yarn test`
-----------

Run mocha, etc. according to a package's `test` script.

The `test:full` variant runs a series of scripts: `lint typecheck test
test_dapps`.

`yarn test_dapps`
-----------------

Runs the `test` script of the embedded `test_dapps` monorepo.

The `test_dapps:ci` and `test_dapps:qa` variants run the `ci` and `qa` scripts
of the embedded `test_dapps` monorepo, respectively.

`yarn typecheck`
----------------

Runs tsc, etc. according to a package's `typecheck` script.

Notes
=====

`npx` is used in some of the top-level and package scripts to ensure the
scripts can run even if `node_modules` is missing.

[`"nohoist"`][nohoist] specifies a couple of embark packages because
[`restrictPath`][restrictpath] is interfering with access to modules that are
located in a higher-up `node_modules`.

All dependencies in `packages/embark-ui` have been made `devDependencies` since
its production build is self-contained.

`packages/embark`'s existing CHANGELOG's formatting has been slightly adjusted
to match the formatting that Lerna will use going forward (entries in the log
haven't been modified).

Lerna will generate a CHANGELOG at the top-level and in each package. Since
we're transitioning to a monorepo, things may look a little wonky with respect
to old entries in `packages/embark/CHANGELOG.md` and going forward we need to
consider how scoping our commits corresponds to member-packages of the
monorepo.

In `packages/embark`, `test` invokes `scripts/test`, which starts a child
process wherein `process.env.DAPP_PATH` is a temporary path that has all of
`packages/embark/dist/test` copied into it, so that paths to test
helpers/fixtures don't need to be prefixed with `dist/test/` and so that a
`.embark` directory doesn't get written into `packages/embark`.

The `"engines"` specified in top-level and packages' `package.json` reflect a
node and npm pair that match (a source of confusion in the past). The pair was
chosen according to the first post v5 npm that's bundled with node. A
`"runtime"` key/object has been introduced in `packages/embark/package.json`
which is used as the basis for specifying the minimum version of node that can
be used to run embark, and that's what is checked by `bin/embark`.

Some changes have been introduced, e.g. in `lib/core/config` and
`lib/utils/solidity/remapImports` so that it's *not* implicitly assumed that
`process.env.DAPP_PATH` / `fs.dappPath()` are the same as
`process.cwd()`. There are probably several++ places where that assumption is
still in effect, and we should work to identify and correct them.

`embark reset` now deletes `embarkArtifacts/` within a dapp root, and
`embarkArtifacts/` is git-ignored.

`lib/core/env` adds all `node_modules` relative to `process.env.EMBARK_PATH` to
`NODE_PATH` so that embark's modules can be resolved as expected whether
embark's `node_modules` have been deduped or are installed in npm's flat
"global style".

`checkDependencies` has been inlined (see `lib/utils/checkDependencies`) and
slightly modified to support dependencies that have been hoisted into a
higher-up `node_modules`, e.g. as part of a yarn workspace. eslint has been
disabled for that script to avoid more involved changes to it.

`test_apps` is not in `packages/embark`; rather, there is `test_dapps` at the
top-level of the monorepo. `test_dapps` is an embedded monorepo, and its `ci` /
`qa` scripts `npm install` embark from freshly built tarballs of the packages
in the outer monorepo and then use that installation to run `embark test` in
the dapps. This should allow us to rapidly detect breakage related to
auto-bumps in transitive dependencies.

[filter options]: https://github.com/lerna/lerna/tree/master/core/filter-options
[embark-readme]: https://github.com/embark-framework/embark/blob/build/lerna/packages/embark/README.md
[fixed-locked]: https://github.com/lerna/lerna#fixedlocked-mode-default
[independent]: https://github.com/lerna/lerna#independent-mode
[npm-scripts]: https://docs.npmjs.com/misc/scripts
[lerna publish]: https://github.com/lerna/lerna/tree/master/commands/publish
[pub-pos]: https://github.com/lerna/lerna/tree/master/commands/publish#positionals
[ver-pos]: https://github.com/lerna/lerna/tree/master/commands/version#positionals
[lerna-json]: https://github.com/embark-framework/embark/blob/build/lerna/lerna.json#L11
[verdaccio]: https://www.npmjs.com/package/verdaccio
[nohoist]: https://github.com/embark-framework/embark/blob/build/lerna/package.json#L52-L55
[restrictpath]: https://github.com/embark-framework/embark/blob/build/lerna/packages/embark/src/lib/core/fs.js#L9
@michaelsbradleyjr
Copy link
Contributor Author

michaelsbradleyjr commented Feb 4, 2019

Thanks for all the helpful feedback and compliments, team! I really appreciate your patience and support as this took shape in the New Year. Thanks especially to @emizzle and @PascalPrecht for the generous amounts of time they spent helping me sort some things out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants