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

build: drop multiple dist targets, always use es2017 #1803

Merged
merged 2 commits into from
Oct 4, 2018

Conversation

bajtos
Copy link
Member

@bajtos bajtos commented Oct 4, 2018

Based on the work in #1636, it's clear that we cannot continue building for multiple ES targets (ES2017 for Node.js 8.x and ES2018 for Node.js 10.x) if we want to use TypeScript Project References. (And we do want to!)

This pull request is removing multi-dist build and switching all packages (including project templates) to always build for ES2017 (Node.js 8.x) only.

  • dist10 is removed completely
  • dist8 is moved to dist
  • build:dist8, build:dist10 and build:dist-all scripts are removed in favor of build:dist
  • remove all usages of lb-mocha's DIST template, use dist instead
  • remove all usages of @loopback/dist-util, load ./dist directly. Keep using @loopback/dist-util inside CLI - we need the version check
  • fix compile-package to correctly treat --outDir path as relative to CWD

@bajtos
Copy link
Member Author

bajtos commented Oct 4, 2018

Grr. Apparently tsc es2017 --outdir dist does not work because dist is considered as absolute to monorepo root, not to the package that involved tsc.

@bajtos bajtos force-pushed the build/single-dist branch from ba4bb84 to 6e6de52 Compare October 4, 2018 11:45
@bajtos bajtos force-pushed the build/single-dist branch 3 times, most recently from 5cfb7e9 to 1f36d7b Compare October 4, 2018 11:59
@bajtos bajtos force-pushed the build/single-dist branch from 1f36d7b to cffa630 Compare October 4, 2018 12:03
@bajtos
Copy link
Member Author

bajtos commented Oct 4, 2018

Uff, we have so many places affected by the design decision to support multiple dist targets! I hope that I found them all, unfortunately it makes the change-set huge.

The pull request is ready for review.

Copy link
Contributor

@raymondfeng raymondfeng left a comment

Choose a reason for hiding this comment

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

👍 Thank you for the effort!

Copy link
Contributor

@marioestradarosa marioestradarosa left a comment

Choose a reason for hiding this comment

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

👍 Excellent @bajtos !

@bajtos bajtos merged commit 9ae93d8 into master Oct 4, 2018
@bajtos bajtos deleted the build/single-dist branch October 4, 2018 14:31
@bajtos bajtos mentioned this pull request Mar 18, 2019
7 tasks
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.

3 participants