[Gradle][K/JS] skip npm install task execution while in offline mode #5410
Conversation
y9san9
force-pushed
the
master
branch
2 times, most recently
from
b13cb98 to
da4dad3
Compare
| @@ -41,8 +41,12 @@ abstract class KotlinNpmInstallTask : | |||
| @get:Internal | |||
| abstract val nodeModules: DirectoryProperty | |||
|
|
|||
| private val isOffline = project.gradle.startParameter.isOffline() | |||
There was a problem hiding this comment.
|
Hi @y9san9. |
… passed to the npm resolution manager
| add("install") | ||
| addAll(args) | ||
| if (logger.isDebugEnabled) add("--verbose") | ||
| if (environment.ignoreScripts) add("--ignore-scripts") | ||
| }.filter { it.isNotEmpty() } |
There was a problem hiding this comment.
Should we leave it there for cases when someone will pass empty arguments in the list?
Before, it was used so we can write code like this:
listOf(
if (something) test else ""
)Later, it was migrated to the approach with mutableList, but filter is still in place.
| @@ -141,12 +141,12 @@ class Npm internal constructor( | |||
| ) { | |||
| val progressLogger = objects.newBuildOpLogger() | |||
| execWithProgress(progressLogger, description, execOps = execOps) { execSpec -> | |||
| val arguments: List<String> = mutableListOf<String>().apply { | |||
| val arguments = buildList { | |||
There was a problem hiding this comment.
These changes are quite unrelated to what I generally do in this PR, so I can either revert them or submit a different PR, or leave them
|
Hello @JSMonk! Thank you for your time. I did what you asked for. |
...ols/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/targets/js/npm/Npm.kt
Outdated
Show resolved
Hide resolved
| @TaskAction | ||
| fun resolve() { | ||
| val args = buildList { | ||
| addAll(args) | ||
| if (isOffline) add("--offline") |
There was a problem hiding this comment.
May I also ask you to introduce a test for this logic?
@ilgonmic, is it possible to somehow emulate the "offline" mode in our Gradle integration test infrastructure?
There was a problem hiding this comment.
Yep, I would love to cover this with tests, but where can I see some examples?
There was a problem hiding this comment.
We have integration tests for the Gradle plugin. You can explore the JS-specific here: https://github.com/JetBrains/kotlin/blob/master/libraries%2Ftools%2Fkotlin-gradle-plugin-integration-tests%2Fsrc%2Ftest%2Fkotlin%2Forg%2Fjetbrains%2Fkotlin%2Fgradle%2FKotlin2JsGradlePluginIT.kt
The rest of the integration tests are located in the same Gradle sub-project
Hello! I am a very naive first-time contributor. I created a ticket in youtrack and assume I can give it a shot and implement it on my own. This is my first iteration which might be completely wrong. So I ask for help to get some directions to me in order if this one is way off.
Right now I just skip execution of npmInstall completely, but I don't know what are the consequences of that. Hope you understand and will give me this ability to try and do this feature.