This repository has been archived by the owner on Dec 5, 2020. It is now read-only.
Support gulp extend without npm search
#266
Comments
|
We actually encounter this problem in our environment. In fact, this is a stumbling block only in the build of our themes in our Concourse Delivery tools. The solution of directly referencing the url of our thongs for example would effectively solve our problem while remaining agnostic of the storage solution. |
wincent
added a commit
that referenced
this issue
Apr 30, 2019
|
I have a draft for this started in: #332 |
wincent
added a commit
that referenced
this issue
Apr 30, 2019
wincent
added a commit
that referenced
this issue
Apr 30, 2019
wincent
added a commit
that referenced
this issue
May 3, 2019
There are a few problems with the way this code was and some are
partially addressed here:
- `modulePackages[module]` is not a terribly revealing name, so we
assign it to a `pkg` variable to at least make it clear that we a
dealing with a single package.
- When we "reduce" the package data (ie. filtering out unwanted fields)
we throw away some information that we'll need later on in order to
save the dependency info to the package.json (in
`_saveDependencies()`) and actually install the dependencies (in
`_installDependencies()`). This is problematic because we need to be
able to distinguish between named dependencies, path-based
dependencies (ie. those with a `__realPath__` magical property) and
URL-based ones (ie. those with ` __packageURL__` property).
- Those two methods don't actually need the "reduced" form of the
package data and could just operate on the original "pkg", so the fix
is to do exactly that.
As a result of this change, we no longer see the theme doctor warning us
about using a "path" when what we actually did was refer to a package
URL:
[13:15:13] Warning: you have dependencies that are installed
from local modules. These should only be used for development
purposes. Do not publish this npm module with those dependencies!
Problems in the code not fixed here:
- We sometimes call `_saveDependencies()` with an object and sometimes
with an array, relying on lodash's `reduce()` method to operate
indistinctly over the collections regardless of the type. This makes the
code harder to reason about and I think it may actually be a bug
albeit an innocuous one.
- We repeatedly read from and write to the package.json file via the
calls to functions on `lfrThemeConfig`. This is a bit of code smell.
For example, we call `setConfig()` to write out the `baseTheme`, then
we call `setDependencies()` to write out the dependcies; this second
call reads the JSON back in that was just written out, modifies it,
and then writes it back out straight away. We are effectively using
the file-system as a slow form of shared memory instead of operating
on in-process data-structures and persisting them at the end.
Closes: #266
wincent
added a commit
that referenced
this issue
May 3, 2019
|
v9.x verision of the PR is #333 |
wincent
added a commit
that referenced
this issue
Jun 27, 2019
feat: teach `gulp extend` to accept a package URL (8.x) (#266)
wincent
added a commit
that referenced
this issue
Jun 27, 2019
feat: teach `gulp extend` to accept a package URL (9.x) (#266)
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
When using Artifactory as your npm registry (and proxy registry),
gulp extendis not working because Artifactory doesn't support npm search syntax yet. Therefore, a feature request has been opened on JFrog: https://www.jfrog.com/jira/browse/RTFACT-18655 (any upvote is welcome).In the meantime, we could maybe add some support to avoid using npm search and refer directly to a package URL. This is just an idea to start a conversation around this issue.
Thanks :^)
The text was updated successfully, but these errors were encountered: