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.

Sign up for GitHub

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

Jump to bottom

Improve Gradle build and fix storybook #13719

Merged
merged 2 commits into from
Jun 13, 2022
Merged

Improve Gradle build and fix storybook #13719

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7f0cd07
Improve Gradle build and fix storybook
timroes Jun 13, 2022
8b7b99b
Run all copy tasks after copyDocker
timroes Jun 13, 2022
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
5 changes: 2 additions & 3 deletions airbyte-webapp-e2e-tests/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,15 @@
plugins {
id "base"
id "com.github.node-gradle.node" version "2.2.4"
id "com.github.node-gradle.node" version "3.3.0"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ℹ️ Updated this to the most recent version, as we already did for the airbyte-webapp project.

}

def nodeVersion = System.getenv('NODE_VERSION') ?: '16.13.0'
def nodeVersion = System.getenv('NODE_VERSION') ?: '16.15.1'

node {
download = true
version = nodeVersion
}


task e2etest(type: NpmTask) {
dependsOn npmInstall
// If the cypressWebappKey property has been set from the outside (see tools/bin/e2e_test.sh)
Expand Down
1 change: 1 addition & 0 deletions airbyte-webapp/.storybook/main.ts → airbyte-webapp/.storybook/main.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ module.exports = {
"@storybook/preset-create-react-app",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ℹ️ I've changed this file to be a .js file again. It seems the typing conflict with ts-node somehow. I was not able to quickly resolve that (custom tsconfig.json, .babelrc, changing this files syntax didn't work). Given that this file anyway has no real typing in it at all, I reverted it back to a .js file.

"storybook-addon-mock/register",
],
staticDirs: ["../public"],
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ℹ️ Move this from the deprecated CLI flag into the config.

webpackFinal: (config) => {
config.resolve.modules.push(process.cwd() + "/node_modules");
config.resolve.modules.push(process.cwd() + "/src");
Expand Down
13 changes: 8 additions & 5 deletions airbyte-webapp/.storybook/withProvider.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import GlobalStyle from "../src/global-styles";
import messages from "../src/locales/en.json";
import { FeatureService } from "../src/hooks/services/Feature";
import { ConfigServiceProvider, defaultConfig } from "../src/config";
import { DocumentationPanelProvider } from "../src/views/Connector/ConnectorDocumentationLayout/DocumentationPanelContext";
import { ServicesProvider } from "../src/core/servicesProvider";
import {
analyticsServiceContext,
Expand Down Expand Up @@ -47,11 +48,13 @@ export const withProviders = (getStory) => (
<ConfigServiceProvider
defaultConfig={defaultConfig}
providers={[]}
>
<FeatureService>
<GlobalStyle />
{getStory()}
</FeatureService>
>
<DocumentationPanelProvider>
<FeatureService>
<GlobalStyle />
{getStory()}
</FeatureService>
</DocumentationPanelProvider>
</ConfigServiceProvider>
</ThemeProvider>
</IntlProvider>
Expand Down
1 change: 0 additions & 1 deletion airbyte-webapp/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,4 @@ FROM ${NGINX_IMAGE} as webapp
EXPOSE 80

COPY bin/build /usr/share/nginx/html
COPY bin/docs /usr/share/nginx/html/docs
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ℹ️ This is no longer needed, since the copyDocs and copyDocAssets tasks now copy their files directly into the build/docker/bin/build/docs folder instead of build/docker/bin/docs as beforehand. Thus bin/build contains the static content now as it's required to be served by nginx.

Copy link
Contributor

Choose a reason for hiding this comment

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

Not needed?

COPY bin/nginx/default.conf.template /etc/nginx/templates/default.conf.template
94 changes: 55 additions & 39 deletions airbyte-webapp/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,78 +5,95 @@ plugins {

def nodeVersion = System.getenv('NODE_VERSION') ?: '16.15.1'

// This array should contain a path to all configs that are common to most build tasks and
// might affect them (i.e. if any of those files change we want to rerun most tasks)
def commonConfigs = [
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ℹ️ Extract common configuration, that might affect all tasks to require a rerun out into a variable.

'.env',
'package.json',
'package-lock.json',
'tsconfig.json',
'.prettierrc.js'
]

node {
download = true
version = nodeVersion
}

npm_run_build {
inputs.files fileTree('public')
inputs.files fileTree('src')
inputs.file 'package.json'
inputs.file 'package-lock.json'
inputs.files commonConfigs
inputs.file '.eslintrc'
inputs.dir 'public'
inputs.dir 'src'

// todo (cgardens) - the plugin seems to ignore this value when the copy command is run. ideally the output would be place in build/app.
outputs.dir project.buildDir
outputs.dir 'build/app'
}

task test(type: NpmTask) {
dependsOn assemble

args = ['run', 'test', '--', '--watchAll=false', '--silent']
inputs.files fileTree('src')
inputs.file 'package.json'
inputs.file 'package-lock.json'
inputs.files commonConfigs
inputs.dir 'src'
}

task licenseCheck(type: NpmTask) {
dependsOn npmInstall

args = ['run', 'license-check']
inputs.file 'package.json'
inputs.file 'package-lock.json'
inputs.files commonConfigs
inputs.file 'scripts/license-check.js'

// The licenseCheck has no outputs, thus we always treat the outpus up to date
// as long as the inputs have not changed
outputs.upToDateWhen { true }
}

task validateLinks(type: NpmTask) {
dependsOn npmInstall

args = ['run', 'validate-links']
inputs.file 'package.json'
inputs.file 'package-lock.json'

// Since the output of this task depends on availability of URLs
// we never want to treat it as "up-to-date" and always want to run it
outputs.upToDateWhen { false }
}

// Make sure to always run a license check after we installed dependencies
npmInstall.finalizedBy licenseCheck
// Validate all links after installing dependencies
npmInstall.finalizedBy validateLinks
assemble.dependsOn npm_run_build
build.finalizedBy test
task buildStorybook(type: NpmTask) {
dependsOn npmInstall
args = ['run', 'build:storybook']

task copyBuild(type: Copy) {
dependsOn copyDocker
inputs.files commonConfigs
inputs.dir '.storybook'
inputs.dir 'public'
inputs.dir 'src'

from "${project.projectDir}/build"
into "build/docker/bin/build"
exclude ".docker"
exclude "docker"
outputs.dir 'build/storybook'
}

task copyBuildOutput(type: Copy) {
dependsOn copyDocker, npm_run_build

from "${project.projectDir}/build/app"
into 'build/docker/bin/build'
}

task copyDocs(type: Copy) {
dependsOn copyDocker
dependsOn copyDocker, copyBuildOutput

from "${project.rootProject.projectDir}/docs/integrations"
into "build/docker/bin/docs/integrations"
//google-ads.md is blocked by Ad Blockers
into "build/docker/bin/build/docs/integrations"
// google-ads.md is blocked by Ad Blockers
rename ('google-ads.md', 'gglad.md')
duplicatesStrategy DuplicatesStrategy.INCLUDE
}

// Copy images that are used in .md integration documentation docs
task copyAssets(type: Copy) {
dependsOn copyDocker
task copyDocAssets(type: Copy) {
dependsOn copyDocker, copyBuildOutput

from "${project.rootProject.projectDir}/docs/.gitbook"
into "build/docker/bin/docs/.gitbook"
into "build/docker/bin/build/docs/.gitbook"
duplicatesStrategy DuplicatesStrategy.INCLUDE
}

Expand All @@ -87,16 +104,15 @@ task copyNginx(type: Copy) {
into "build/docker/bin/nginx"
}

copyBuild.dependsOn npm_run_build
copyNginx.dependsOn npm_run_build
copyDocs.dependsOn npm_run_build
copyAssets.dependsOn npm_run_build
assemble.dependsOn copyDocs
copyDocker.dependsOn(npm_run_build)
// Those tasks should be run as part of the "check" task
check.dependsOn validateLinks, licenseCheck, test

build.dependsOn buildStorybook

Task dockerBuildTask = getDockerBuildTask("webapp", "$project.projectDir", "$rootProject.ext.version", "$rootProject.ext.image_tag")
dockerBuildTask.dependsOn(copyBuild)
dockerBuildTask.dependsOn(copyDocker)
dockerBuildTask.dependsOn(copyBuildOutput)
dockerBuildTask.dependsOn(copyNginx)
dockerBuildTask.dependsOn(copyDocs)
dockerBuildTask.dependsOn(copyAssets)
dockerBuildTask.dependsOn(copyDocAssets)
assemble.dependsOn(dockerBuildTask)
5 changes: 3 additions & 2 deletions airbyte-webapp/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,12 @@
},
"scripts": {
"start": "react-scripts start",
"build": "react-scripts build",
"build": "BUILD_PATH='./build/app' react-scripts build",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ℹ️ Move the actual react build output to build/app instead of build so we don't mix it with the docker and storybook output.

"test": "react-scripts test",
"test:coverage": "npm test -- --coverage --watchAll=false",
"format": "prettier --write 'src/**/*.{ts,tsx}'",
"storybook": "start-storybook -p 9009 -s public --quiet",
"storybook": "start-storybook -p 9009 --quiet",
"build:storybook": "build-storybook -o 'build/storybook'",
"lint": "eslint --ext js,ts,tsx src",
"license-check": "node ./scripts/license-check.js",
"generate-client": "orval",
Expand Down