Skip to content
This repository has been archived by the owner on Dec 2, 2021. It is now read-only.

Exclude build artifacts from package via gitignore #7

Open
4 tasks
kwasi opened this issue Jan 21, 2020 · 2 comments
Open
4 tasks

Exclude build artifacts from package via gitignore #7

kwasi opened this issue Jan 21, 2020 · 2 comments

Comments

@kwasi
Copy link
Contributor

kwasi commented Jan 21, 2020

Background

As of now I haven't gotten kubeflow/frontend to build successfully in both kubeflow/metadata and kubeflow/pipelines without running into conflicts with the way they both enforce TS config and style rules.

kubeflow/pipelines uses react-scripts-ts
kubeflow/frontend uses reacts-scripts vanilla

These are the ts configs:

https://github.com/kubeflow/metadata/blob/master/frontend/tsconfig.json
https://github.com/kubeflow/pipelines/blob/master/frontend/tsconfig.json

These config files are enforced by react-scripts-ts:

https://github.com/kubeflow/pipelines/blob/master/frontend/tsconfig.prod.json
https://github.com/kubeflow/pipelines/blob/master/frontend/tsconfig.test.json

Problem

This repo includes checked in ES5 code and points instead of compiling as a postinstall step.

Workaound

NOTE: You have to run npm build after making changes in order for the changes to be reflected in dependent repos. npm run build:watch will allow changes to be reflected during development.

Fix Tasks:

  • Update this issue with an output of the build conflict
  • Unify kubeflow/frontend and kubeflow/pipelines TS Config and TS Lint rules so they aren't in conflict.
  • Run npm build as postinstall
  • Add build/ to .gitignore and remove build artifcats
@Bobgy
Copy link
Contributor

Bobgy commented Feb 24, 2020

@kwasi Have you configured using tsdx as template for this repo?
https://github.com/jaredpalmer/tsdx

It is zero config like create-react-app, but for typescript library.

Issues like this one and a lot more are solved out of box by it.

The issue you mentioned of different tsconfig.json causing a problem is actually caused by https://github.com/kubeflow/frontend/blob/master/index.d.ts. Because usually when a library is built, it should build both compiled js bundles and compiled typings in *.d.ts format, so they are just typings. So clients only use *.d.ts typing and won't care about ts implementation details in specific ts files.
(context, I was trying to migrate to create-react-app in https://github.com/kubeflow/pipelines/pull/3156/files#diff-64282092d7b77f064e3f06a569429a99R25, but have to disable one extra rule because @kubeflow/frontend is breaking it. I got an error like /Users/gongyuan/kfp/pipelines/frontend/node_modules/@kubeflow/frontend/src/mlmd/LineageView.tsx TypeScript error in /Users/gongyuan/kfp/pipelines/frontend/node_modules/@kubeflow/frontend/src/mlmd/LineageView.tsx(92,11): Property 'artifactTypes' has no initializer and is not definitely assigned in the constructor. TS2564)

Above is also solved in tsdx too.

It has three templates:

  • typescript
  • typescript + react
  • typescript + react + storybook

I was a fan of storybook, but haven't been maintaining a component library for a while. I think it's worth a try, so I recommend you set it up with typescript + react + storybook template, and it's up-to-you when to use more about storybook.

@kwasi
Copy link
Contributor Author

kwasi commented Feb 26, 2020

@Bobgy Thanks, we're not using it here, but that looks like a good library. I'll give it a closer look.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants