Load towers dynamically #169
Conversation
Codecov Report
@@ Coverage Diff @@
## master #169 +/- ##
==========================================
+ Coverage 92.19% 92.35% +0.15%
==========================================
Files 83 85 +2
Lines 1051 1072 +21
Branches 177 179 +2
==========================================
+ Hits 969 990 +21
Misses 67 67
Partials 15 15
Continue to review full report at Codecov.
|
olistic
force-pushed
the
load-towers
branch
2 times, most recently
from
d59b1b0
to
1b746ac
Compare
| ); | ||
| const towerPaths = towerPackageJsonPaths.map(path.dirname); | ||
| const towerRequirePaths = towerPaths.map(towerPath => | ||
| resolve.sync(towerPath, { basedir: towerSearchDir }), |
There was a problem hiding this comment.
Hi @olistic 👋
resolve.sync maps paths to directories into paths to the actual js files, which you will need later when calling require. In most cases this mapping simply adds index.js at the path's the end, however, if you dir's package.js says "main": "something.js", the resolved path becomes dir/something.js. E.g.:
There was a problem hiding this comment.
@kachkaev My understanding is that require() has the same resolution algorithm built-in, so I guess I still want to call resolve to be able to pass the basedir and make sure the module I'm loading is the one I found previously with globby (and not a different one in the default node_modules recursive path)?
| resolve.sync(towerPath, { basedir: towerSearchDir }), | ||
| ); | ||
| const towers = towerRequirePaths.map( | ||
| towerRequirePath => require(towerRequirePath), // eslint-disable-line global-require, import/no-dynamic-require |
There was a problem hiding this comment.
Is the require() call safe? I saw this same thing being done like this in other places:
eval('require')(towerRequirePath)There was a problem hiding this comment.
eval('require') is used in Prettier because that library is built with Rollup. eval prevents towerRequirePath from an attempt to be glued together with the current module. If you don't have Rollup in your workflow, feel free to just require(towerRequirePath).
Hope this helps! 👋
There was a problem hiding this comment.
Ah! Makes perfect sense, thanks!
olistic
force-pushed
the
load-towers
branch
9 times, most recently
from
4b71204
to
dce4a2d
Compare
Instead of tower name. This change is needed to prevent conflicts when loading profiles. Tower names can be duplicated, whereas identifier uniqueness is enforced by npm (because identifiers are derived from the package name).
This adds support for community towers, as well as official towers that don't ship with the game. WarriorJS CLI searches for towers (
@warriorjs/tower-*andwarriorjs-tower-*) in the nearest parentnode_modulesdirectory. This should work for both global and local installations of the CLI.Closes #100
Fixes #168