Prior to this PR, typing `SimpleStorage.methods.` in to the cockpit console would not show autosuggestions for the contract methods.
Contract names are case sensitive in the VM (VM2). Update the `Suggestions.getSuggestions` method to be case-sensitive when evaluating `Object.keys(<contract class name>.methods)`.
`EmbarkJS.Blockchain.connectConsole` has a potential race condition with both `cb` and `doneCb` firing at the end of the method.
This PR is an attempt to fix that by first awaiting the `cb`, then finally calling `doneCb`, as suggested by @michaelsbradleyjr in https://github.com/embark-framework/embark/pull/1319#discussion_r256850820
This PR introduces a large number of changes (for that, I am very sorry). Breaking this up in to smaller PR's was causing tests to fail, and the likelihood of them getting merged diminished. There a couple of PRs this PR closes, and as such, I have kep the original commits to preserve the history. The first two (of three) commits are the close PRs, and the last is the glue bringin it all together.
The main goal of this PR is to fix the fragility of the tests with EmbarkJS, however in doing so, a number of recent features have been updated:
Remapping of imports still had a few edge cases that needed to be ironed out, as well as have unit tests created for them. More details of the changes an be seen in the closed PR (below).
The main issue with VM2 was running the code generated EmbarkJS code inside the VM, and getting EmbarkJS contracts out of the VM and available to the tests. This fixed issues where ENS may not have been available to the tests. Notable additions include adding `EmbarkJS.Blockchain.connectTests` to the tests lifecycle, and ensuring `EmbarkJS` is only every required in the console module and not used or passed around elsewhere.
As mentioned above, the main issue with the tests in the context of a monorepo and with embark running as module inside the dapp’s `node_modules`, there were issues getting the correct contract state available inside of the tests. For some reason, this particular case was causing the tests to fail, with ENS never being available (assuming this to be an issue with `EmbarkJS.Blockchain.connect()` never being called). The main fix for this came with passing `web3` as an option in to `EmbarkJS.Blockchain.setProvider()`.
---
1. https://github.com/embark-framework/embark/pull/1286
2. https://github.com/embark-framework/embark/pull/1275
Go to bottom for details
---
There are few known issues with this PR. Instead of trying to fix all of them with this PR, I was hoping to get these issues tackled in separate PRs.
1. `deployIf` directive on contracts defined in the config are not working, however the tests are passing. The issue is that if `ContractA` has a `deployIf` set to `!!ContractB.options.address`, it means that it will not deploy if `ContractB` is not deployed. However, it appears that `ContractA` is attempted to be deployed prior to `ContractB` being deployed, and therefore `ContractA` fails to deploy. Instead, because `ContractA` depends on `ContractB`, `ContractB` should be deployed before `ContractA`.
2. `embark test --node embark` does not seem to be functioning for reasons unknown.
3. Remix tests: Currently there is support for adding contract tests that get process by `remix-tests`, however, there is an error that I believe is not due to embark, but due to the Assert library. For example, if we add a `test/remix_test.sol` to the `test_app` with the following content:
```
pragma solidity ^0.4.24;
import "remix_tests.sol";
import "../app/contracts/simple_storage.sol";
contract SimpleStorageTest {
SimpleStorage simpleStorage;
function beforeAll() public {
simpleStorage = new SimpleStorage(100);
}
function initialValueShouldBeCorrect() public {
return Assert.equal(
100,
simpleStorage.storedData,
"stored data is not what I expected"
);
}
}
```
After compilation, we would get the error:
```
remix_test.sol:14:12: TypeError: Member "equal" not found or not visible after argument-dependent lookup in type(library Assert)
return Assert.equal(
^—————^
```
---
This branch is based off of ()`refactor/embarkjs-in-monorepo`)[https://github.com/embark-framework/embark/tree/refactor/embarkjs-in-monorepo], which does not have passing tests due to the `EmbarkJS` ENS issue mentioned above. However, you should (hopefully) see the tests passing in this branch, meaning if both branches are merged, the tests should be passing.
Related PRs: https://github.com/embark-framework/embark-solc/pull/24
---
Changes include:
1. Add unit tests for recursively remapping imports
2. Handle plugin contracts correctly
3. Allow `prepareForCompilation` to be called from `File`, allowing for external compilers, like `embark-solc` to call this function before compilation.
4. Add flattened remappings to `File` that gets prepared for compilation (ie has it's imports remapped)
5. Return remapped contract content when file type is http. Previously this was broken, as always freshly downloaded (original) content was returned.
6. Handle additional cases for `custom` and http file types.
This PR was tested with:
- `embark` unit tests
- `embark` test_app
- `embark` test_app with `embark-solc` plugin
- `embark` test_app with `embark-flattener` plugin
- `Giveth/lpp-campaign`
Related change to get `embark-solc` up-to-date` with these changes: https://github.com/embark-framework/embark-solc/pull/24
When embark was running as module inside the dapp’s `node_modules`, the tests were failing due to several issues:
1. `web3` was not being set in the global namespace of vm2. `EmbarkJS.Blockchain.setProvider` was therefore failing because it relies on `global.web3` to be set. I guess somehow this works when the test app was running in a child tree of the executing program. maybe this is a security feature of vm2, but i’m not sure.
2. `embarkjs` provider code being injected to the vm twice. This really was the initial point of failure, as this piece of code is requiring embarkjs, so i’m assuming, but again not sure, that maybe it was getting a second instance of `EmbarkJS` which did not have it’s providers set up correctly (hence the error with `ENS provider not set`).
Fixes for those issues in this PR:
1. To circumvent the web3 issue, we are now setting `global.web3` for tests only (the global web3 accessible in the tests), and `web3` is now explicitly passed in to `EmbarkJS.Blockchain.setProvider`
2. To fix the `embarkjs` code being called twice, we are not re-injecting this code to the VM during test initialisations
Changes include:
- Add unit tests for recursively remapping imports
- Handle plugin contracts
- Allow `prepareForCompilation` to be called from `File`. Allows external compilers, like `embark-solc` to call this.
- Add flattened remappings to file getting prepared for compilation
- Return remapped contract content when file type is http
- Add check allowing files to be remapped/prepared multiple times
When embark was running as module inside the dapp’s `node_modules`, the tests were failing due to several issues:
1. `web3` was not being set in the global namespace of vm2. `EmbarkJS.Blockchain.setProvider` was therefore failing because it relies on `global.web3` to be set. I guess somehow this works when the test app was running in a child tree of the executing program. maybe this is a security feature of vm2, but i’m not sure.
2. `embarkjs` provider code being injected to the vm twice. This really was the initial point of failure, as this piece of code is requiring embarkjs, so i’m assuming, but again not sure, that maybe it was getting a second instance of `EmbarkJS` which did not have it’s providers set up correctly (hence the error with `ENS provider not set`).
Fixes for those issues in this PR:
1. To circumvent the web3 issue, we are now setting `global.web3` for tests only (the global web3 accessible in the tests), and `web3` is now explicitly passed in to `EmbarkJS.Blockchain.setProvider`
2. To fix the `embarkjs` code being called twice, we are not re-injecting this code to the VM during test initialisations
Packages' `"clean"` scripts should run regardless of whether `node_modules` is
in place at the root and/or package levels, but the `embarkjs` package was
missing a needed `npx` in front of `rimraf` in its script.
Also, bump its devDeps version of rimraf to be in line with the rest of
`packages/*` and `test_dapps/*`.
If a function receives a callback argument then it should not return a promise
if the caller's callback will be invoked. Both invoking a callback and
returning a promise can lead to at best confusion (in code review and at
runtime) and at worst non-deterministic behavior, such as race
conditions. Also, a caller supplying a callback may not handle a returned
promise, leading to unhandled rejection errors.
Refactor all readily identified functions where a callback argument can be
supplied but the function returns a promise regardless. Make use of
`callbackify` and `promisify` where it made sense to do so during the
refactoring. Some callsites of the revised functions may have been accidentally
overlooked and still need to be updated. Some functions that take callback
arguments may execute them synchronously, at odds with control flow of a
returned promise (if a callback wasn't supplied). Such cases should be
identified and fixed so that asynchronous behavior is fully consistent whether
the caller supplies a callback or receives a promise.
Make sure promises that pass control flow to a callback ignore rejections,
since those should be handled by the callback.
Don't return promise instances unnecessarily from async functions (since they
always return promises) and change some functions that return promises to async
functions (where it's simple to do so).
Whisper was using an ad hoc promise-like `messageEvents` object. However, that
object behaved more like an observable, since promises either resolve or
reject, and only do so one time. `messageEvents` was also intertwined with
callbacks. Replace `messageEvents` with RxJS Observable. `listenTo` now returns
Observable instances and callers can subscribe to them.
`Blockchain.connect` of embarkjs could suffer from a race condition where tasks
associated with `execWhenReady` might be ongoing when `connect`'s returned
promise resolves/rejects (or a caller supplied callback fires). Attempt to
ensure that returned-promise / supplied-callback control flow proceeds only
after `execWhenReady` tasks have finished. The control flow involved
is... rather involved, and it could use some further review and refactoring.
Bump webpack and the hard-source-plugin for webpack.
[util]: https://www.npmjs.com/package/util
Prior to this commit Embark had a bug where, when running `$ embark run` inside
an Embark project that was freshly cloned or reset, it'd get stuck at loading
the Solidity compiler.
However, this only seemed to happen when Embark was used with its dashboard. When
running
```
$ embark run --nodashboard
```
instead, Embark loaded Solidity successfully and moved on with compilation.
This bug pretty much boiled down to `SolidityProcess` not receiving a valid `Logger`
dependency when instantiated via `SolcW`. The reason this was happening
is that we were sending the needed dependencies via `ProcessLauncher.send()`, which
serializes/deserializes its sent data.
```
solidityProcessLauncher.send({action: 'init', options: { logger: this.logger }});
```
And inside `SolidityProcess`
```
process.on('message', (msg) => {
if (msg.action === "init") {
solcProcess = new SolcProcess(msg.options);
}
...
}
```
`SolcProcess` passes its `logger` instance down to `longRunningProcessTimer` which
uses methods on `Logger`.
However, since the data had been serialized, all prototype methods on the data is
gone, resulting in an error, which made Embark stop.
**Why was this only a problem when using the dashboard?**
Turns out we've only relied on `Logger.info()` in case the dashboard is used.
refactor(@embark) add fs to plugin api; change (most) modules to use it instead of coupling them with require
refactor(@embark) add fs to plugin api; change (most) modules to use it instead of coupling them with require
refactor(@embark) add fs to plugin api; change (most) modules to use it instead of coupling them with require
refactor(@embark) add fs to plugin api; change (most) modules to use it instead of coupling them with require
refactor(@embark) add fs to plugin api; change (most) modules to use it instead of coupling them with require
refactor(@embark) add fs to plugin api; change (most) modules to use it instead of coupling them with require
Prior to this commits `this.contractsFiles` kept growing at run time
for every file change that was detected by Embark.
The reason for that was a bug in a condition that determines whether
a subset of contract files needed to be added to memory or not.
In addition to that we kept pushing externally loaded contract files into
memory, even if they already existed in memory.
Don't import git history of embark-framework/EmbarkJS, simply copy over the
sources. Modify `package.json`, etc. re: being situated in the monorepo.
Make use of the root babel config but extend with
`packages/embarkjs/.babelrc.js`.
Build `test/` scripts into `build-test/` and git-ignore `build-test/`.
Revise `Blockchain.connect()` so that if the caller supplies a callback then a
promise is not returned.
Revise tests to test `Blockchain.connect()` usage with and without a callback.
Relative `"include"` path/s in a `tsconfig.json` are relative to the directory
in which the config is located, so each monorepo package that uses typescript
needs its own `tsconfig.json` that `"extends"` the root config.
For now, disable the `"typecheck"` script in embark-graph and embark-vyper, but
leave the relevant pieces in place (devDeps, configs) so that when they're
converted to TypeScript the scripts can simply be re-enabled. The problem is
that if no `.ts` files are present in the path/s specified in `"include"` of
`tsconfig.json`, tsc will exit with error.
refactor(@embark/embark-compiler) move compiler to its own module
This is a combination of 2 commits.
refactor(@embark/embark-compiler) move compiler to its own module
refactor(@embark/embark-compiler) move compiler to its own module
Update packages/embark-compiler/package.json
Co-Authored-By: iurimatias <iuri.matias@gmail.com>
Update packages/embark-compiler/package.json
Co-Authored-By: iurimatias <iuri.matias@gmail.com>
Update packages/embark-compiler/package.json
Co-Authored-By: iurimatias <iuri.matias@gmail.com>
refactor(@embark/embark-compiler) move compiler to its own module
refactor(@embark/embark-compiler) move compiler to its own module
refactor(@embark/embark-compiler) move compiler to its own module
refactor(@embark/embark-compiler) move compiler to its own module
Move `packages/embark`'s eslint config into the monorepo root.
Remove `"plugin:react/recommended"` since it's not commonly needed.
Put a `"eslintConfig"` config key/object in `packages/embark`'s `package.json`
that directs eslint to extend the root config; other `packages/*` can do
likewise.
Setup a `babel.config.js` in the root of the monorepo to be used by
`packages/*`. It won't be used by some packages, e.g. `packages/embark-ui`, but
most of them should use it instead of rolling their own.
Allow for package-level modifications by specifying `babelrcRoots` in the root
`babel.config.js`
Use the babel `--root-mode upward` option in `packages/embark`'s `build`
script. Other packages intending to use the common config should do likewise.
Use a `.babelrc.js` in `packages/embark` to supply the package-specific
`ignore` settings.
Make packages used by the common config devDeps of the root.
Extract babel-related devDeps from `packages/embark`, but don't extract the
non-dev deps since those are used by embark's pipeline in a production
install. Normally, it should only be necessary to have `@babel/cli` and
`@babel/core` in devDeps, and possibly `@babel/runtime-corejs2` in deps, plus
any package-specific babel-related dev/deps. Once we deprecate the pipeline, we
can finish the extraction.
Use `ncu -f '/babel/' -u` to bump the versions of all babel-related deps in the
root and in `packages/embark`. We get better space/time savings from the yarn
workspace when versions match.
fix typings
WIP: refactor
WIP: just needed an annotation 🎉 thanks @emizzle!
WIP: don't pack a tarball since it's a private package
WIP: no need to list as a devDep
Put common patterns we expect to use across most `packages/*` into the
top-level `.gitignore`. If there are package-specific patterns they can go in
that package's `.gitignore`. For example, `packages/embark-ui` ignores its
`build/` directory while commonly we ignore the packages' `dist/` directories.
Yarn merges `.yarnrc` from the current working directory with `.yarnrc` files
higher in the directory tree, so that file isn't needed in each package.
Unfortunately, npm doesn't do the same for `.npmrc` files, so a similar cleanup
isn't possible.
Add version info to `test_app/extensions/embark-service/package.json` to that
`yarn install` can work correctly if manually invoked in `test_app`.
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
emizzle
Jonathan Rainville
Michael Bradley, Jr
Andre Medeiros
Pascal Precht
Anthony Laibe
Iuri Matias