Skip to content
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

Don't publish legacy js artifacts #4591

Merged
merged 4 commits into from Dec 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/benchmarks.yml
Expand Up @@ -21,7 +21,7 @@ jobs:
ulimit -c unlimited
# Workaround an issue where kotlinNpmInstall outputs
# 'Resolving NPM dependencies using yarn' returns 137
./gradlew compileKotlinJsIr compileKotlinJsLegacy
./gradlew compileKotlinJs
./gradlew --stop
- run: |
# Build the libs
Expand Down
6 changes: 3 additions & 3 deletions .github/workflows/pr.yml
Expand Up @@ -26,7 +26,7 @@ jobs:
ulimit -c unlimited
# Workaround an issue where kotlinNpmInstall outputs
# 'Resolving NPM dependencies using yarn' returns 137
./gradlew compileKotlinJsIr compileKotlinJsLegacy
./gradlew compileKotlinJs
./gradlew --stop
./gradlew ciTestsGradle
- name: Collect Diagnostics
Expand All @@ -52,7 +52,7 @@ jobs:
ulimit -c unlimited
# Workaround an issue where kotlinNpmInstall outputs
# 'Resolving NPM dependencies using yarn' returns 137
./gradlew compileKotlinJsIr compileKotlinJsLegacy --stacktrace
./gradlew compileKotlinJs --stacktrace
./gradlew --stop
./gradlew ciTestsNoGradle --stacktrace
- name: Collect Diagnostics
Expand All @@ -78,7 +78,7 @@ jobs:
ulimit -c unlimited
# Workaround an issue where kotlinNpmInstall outputs
# 'Resolving NPM dependencies using yarn' returns 137
./gradlew compileKotlinJsIr compileKotlinJsLegacy
./gradlew compileKotlinJs
./gradlew --stop
./gradlew -p tests ciBuild
- name: Collect Diagnostics
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/push.yml
Expand Up @@ -23,7 +23,7 @@ jobs:
ulimit -c unlimited
# Workaround an issue where kotlinNpmInstall outputs
# 'Resolving NPM dependencies using yarn' returns 137
./gradlew --no-build-cache compileKotlinJsIr compileKotlinJsLegacy
./gradlew --no-build-cache compileKotlinJs
./gradlew --stop
./gradlew --no-build-cache ciBuild
./gradlew --stop
Expand Down
8 changes: 2 additions & 6 deletions build-logic/src/main/kotlin/Mpp.kt
@@ -1,6 +1,5 @@
import org.gradle.api.Project
import org.jetbrains.kotlin.gradle.dsl.KotlinMultiplatformExtension
import org.jetbrains.kotlin.gradle.plugin.KotlinJsCompilerType
import org.jetbrains.kotlin.gradle.plugin.getKotlinPluginVersion
import org.jetbrains.kotlin.gradle.plugin.mpp.KotlinNativeTarget

Expand Down Expand Up @@ -37,7 +36,6 @@ fun Project.configureMppDefaults(withJs: Boolean, withLinux: Boolean, withAndroi
withLinux = withLinux,
appleTargets = enabledAppleTargets,
withAndroid = withAndroid,
kotlinJsCompilerType = KotlinJsCompilerType.BOTH,
newMemoryManager = null
)
}
Expand All @@ -59,7 +57,6 @@ fun Project.configureMppTestsDefaults(
withLinux = false,
withAndroid = false,
appleTargets = appleTargets,
kotlinJsCompilerType = KotlinJsCompilerType.IR,
newMemoryManager = newMemoryManager
)
}
Expand All @@ -70,7 +67,6 @@ fun Project.configureMpp(
withLinux: Boolean,
withAndroid: Boolean,
appleTargets: Collection<String>,
kotlinJsCompilerType: KotlinJsCompilerType,
newMemoryManager: Boolean?,
) {
val kotlinExtension = extensions.findByName("kotlin") as? KotlinMultiplatformExtension
Expand All @@ -83,7 +79,7 @@ fun Project.configureMpp(
}

if (enabledJs && withJs) {
js(kotlinJsCompilerType) {
js(IR) {
nodejs {
testTask {
useMocha {
Expand Down Expand Up @@ -135,7 +131,7 @@ private fun KotlinMultiplatformExtension.createAndConfigureAppleTargets(presetNa
if (presetNames.isEmpty()) {
return
}

if (System.getProperty("idea.sync.active") != null) {
// Early return. Inside intelliJ, only configure one target
targetFromPreset(presets.getByName(hostTarget), "apple")
Expand Down
Expand Up @@ -49,6 +49,12 @@ object BuildDirLayout {
)
}

internal fun legacyJsTargetCheck(project: Project): Provider<RegularFile> {
return project.layout.buildDirectory.file(
"generated/checks/apollo/legacyJsTargetCheck"
)
}

internal fun duplicatesCheck(project: Project, service: Service): Provider<RegularFile> {
return project.layout.buildDirectory.file(
"generated/checks/apollo/${service.name}/duplicatesCheck"
Expand Down
Expand Up @@ -15,7 +15,6 @@ import com.apollographql.apollo3.compiler.hooks.ApolloCompilerKotlinHooks
import com.apollographql.apollo3.compiler.hooks.internal.AddInternalCompilerHooks
import com.apollographql.apollo3.compiler.hooks.internal.ApolloCompilerJavaHooksChain
import com.apollographql.apollo3.compiler.hooks.internal.ApolloCompilerKotlinHooksChain
import com.apollographql.apollo3.compiler.toUsedCoordinates
import com.apollographql.apollo3.gradle.api.AndroidProject
import com.apollographql.apollo3.gradle.api.ApolloAttributes
import com.apollographql.apollo3.gradle.api.ApolloExtension
Expand All @@ -41,6 +40,7 @@ import org.gradle.api.publish.maven.MavenPublication
import org.gradle.api.tasks.TaskProvider
import org.gradle.util.GradleVersion
import org.jetbrains.kotlin.gradle.dsl.KotlinMultiplatformExtension
import org.jetbrains.kotlin.gradle.targets.js.KotlinJsTarget
import java.io.File
import java.util.concurrent.Callable
import javax.inject.Inject
Expand Down Expand Up @@ -131,7 +131,7 @@ abstract class DefaultApolloExtension(

project.afterEvaluate {
if (registerDefaultService) {
val packageNameLine = if (defaultService.packageName.isPresent) {
val packageNameLine = if (defaultService.packageName.isPresent) {
"packageName.set(\"${defaultService.packageName.get()}\")"
} else {
"packageNamesFromFilePaths()"
Expand Down Expand Up @@ -184,6 +184,16 @@ abstract class DefaultApolloExtension(
}

maybeLinkSqlite()

checkForLegacyJsTarget()
}
}

private fun checkForLegacyJsTarget() {
val kotlin = project.extensions.findByName("kotlin") as? KotlinMultiplatformExtension
val hasLegacyJsTarget = kotlin?.targets?.any { target -> target is KotlinJsTarget && target.irTarget == null } == true
check(!hasLegacyJsTarget) {
"Apollo: LEGACY js target is not supported by Apollo, please use IR."
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to add something in the CHANGELOG about this, certainly link to the Kotlin changelog too

}
}

Expand Down
Expand Up @@ -47,4 +47,4 @@ object ModelNames {
* A resolvable configuration that will collect all metadata for a given service name
*/
fun duplicatesConsumerConfiguration(service: Service) = camelCase("apollo", service.name, "DuplicatesConsumer")
}
}
Expand Up @@ -281,6 +281,19 @@ class ServiceTests {
}
}

@Test
fun `legacy js target is not supported`() {
withTestProject("legacyJsTarget") { dir ->
var exception: Exception? = null
try {
TestUtils.executeTask("generateApolloSources", dir)
} catch (e: UnexpectedBuildFailure) {
exception = e
Truth.assertThat(e.message).contains("Apollo: LEGACY js target is not supported by Apollo, please use IR.")
}
assertNotNull(exception)
}
}

@Test
fun `operationOutput uses same id as the query`() {
Expand Down
@@ -0,0 +1,16 @@
plugins {
alias(libs.plugins.kotlin.multiplatform)
alias(libs.plugins.apollo)
}

kotlin {
js(LEGACY) {
browser()
}
}

apollo {
service("service") {
packageNamesFromFilePaths()
}
}
@@ -0,0 +1 @@
apply(from = "../../../../gradle/test.settings.gradle.kts")
@@ -0,0 +1,3 @@
type Query {
random: Int!
}