-
Notifications
You must be signed in to change notification settings - Fork 77
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[GX-17918] Refactor resolve #64
Conversation
if (resolveFrom) { | ||
this._resolveFrom = Array.isArray(resolveFrom) ? resolveFrom : [resolveFrom]; | ||
} | ||
this._require = _require || require; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 two most important features to preserve.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed the guts of @gasket/resolve
(but not the upstream changes). The new separation of concerns feels a lot cleaner. I'm 👍 with a review from @DullReferenceException and/or another person with deep knowledge of @gasket/plugin-engine
packages/gasket-resolve/package.json
Outdated
"test:coverage": "jest --coverage", | ||
"posttest": "npm run lint" | ||
"posttest": "npm run lint", | ||
"docs": "jsdoc2md -t jsdoc2md/README.hbs lib/*.js > README.md; echo" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you need an empty echo
here?
Summary
This is a refactor to
@gasket/resolve
necessary for consistent resolve patterns in cli, engine, presets, and metadata-plugin. This PR includes several fixes and changes.I will highlight the improved separation of concerns changes next.
Changelog
Engine does not implement configured plugin loading
._plugin
of the engine, which was only a snapshot of useful info gathered by resolve.@gasket/resolve
which allows non-engine uses.Engine does not initiate metadata
@gasket/metadata-plugin
.Presets do not resolve their own plugins
@gasket/resolve
, major changes to this package which would be used by the engine and cli would enter a broken state.@gasket/resolve
, used by the engine and cli.Engine and metadata-plugin do not resolve package paths
--preset-path
with the cli.@gasket/resolve
Test Plan
With the refactor, I was able to write more granular unit tests.