Skip to content

Conversation

@feiming
Copy link
Contributor

@feiming feiming commented Sep 4, 2020

Change tsc output-dir to build

conflicting dist dir usage, it's preventing the plugin from being built outside of backstage monorepo

✔️ Checklist

  • All tests are passing yarn test
  • Screenshots attached (for UI changes)
  • Relevant documentation updated
  • Prettier run on changed files
  • Tests added for new functionality
  • Regression tests added for bug fixes

@feiming feiming changed the title Conflict dist Change tsc output dir tor build Sep 4, 2020
@feiming feiming changed the title Change tsc output dir tor build Change tsc output dir to build Sep 4, 2020
@feiming feiming marked this pull request as ready for review September 4, 2020 09:17
@feiming feiming requested a review from a team as a code owner September 4, 2020 09:17
@Rugvip
Copy link
Member

Rugvip commented Sep 4, 2020

Yep, this is an issue indeed, but tbh I don't using a separate folder is what we want to do in this case.

I'm imagining it would be possible to switch the behaviour a bit for single-plugin repos, where we put the types inside dist/.types or something like that, and then exclude that folder when publishing.

Do you want to take a stab at that? Or perhaps open an issue where we can discuss different approaches.

@feiming
Copy link
Contributor Author

feiming commented Sep 4, 2020

yarn build will delete dist on run. We'll still face the same issue where yarn build can't find types

@Rugvip
Copy link
Member

Rugvip commented Sep 4, 2020

@feiming Right, we'd have to do some changes to the build process in the CLI that makes it aware of whether it's executing in single-plugin or monorepo mode, and then act accordingly. Afaik a safe way to check whether we're in a monorepo or not is to check paths.targetDir === paths.targetRoot.

@Rugvip
Copy link
Member

Rugvip commented Sep 4, 2020

Another option would be to do this, and change the build accordingly, but I would prefer a name like dist-types or something like that. build and dist are just two names for the same concept, and we settled on using dist and should stick to that imo. Prefixing with dist- (instead of something like types-dist) ensures that the folder is located next to the dist folder in directory views, which I think is conventient.

@feiming
Copy link
Contributor Author

feiming commented Sep 4, 2020

I'm fine with any name. I'm new to typescript, I have no preference. I'll switch to dist-types.

btw, is E2E test broken? I tried running it on master and got the same result.

@Rugvip
Copy link
Member

Rugvip commented Sep 4, 2020

e2e tests require that you run yarn tsc and yarn build in the root first. That should be working fine in master afaik.

@feiming feiming changed the title Change tsc output dir to build Change tsc output dir to dist-types Sep 4, 2020
@feiming feiming changed the title Change tsc output dir to dist-types WIP: Change tsc output dir to dist-types Sep 5, 2020
@feiming feiming marked this pull request as draft September 5, 2020 08:46
@feiming feiming marked this pull request as ready for review September 6, 2020 01:17
@feiming feiming changed the title WIP: Change tsc output dir to dist-types WIP: Sep 6, 2020
@feiming feiming changed the title WIP: Change tsc output dir to dist-types Sep 6, 2020
Copy link
Member

@Rugvip Rugvip left a comment

Choose a reason for hiding this comment

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

Looks good, thanks! 😁

@Rugvip Rugvip merged commit 1b341aa into backstage:master Sep 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants