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

lerna link convert #1730

Merged
merged 25 commits into from
Jul 19, 2019
Merged

lerna link convert #1730

merged 25 commits into from
Jul 19, 2019

Conversation

goto-bus-stop
Copy link
Contributor

This installs dependencies of all packages, the website, and all examples into the root node_modules folder. After an npm install, no further lerna bootstrap is required.

Internal dependencies within the repository now use file: paths. This way npm can do all of it by itself. When doing a lerna publish, lerna temporarily rewrites those to the actual version numbers.

Note that when npm manages all dependencies, devDependencies of subpackages are not installed. This is because the subpackages like @uppy/core are themselves seen as dependencies by npm. dev dependencies need to be added to the root package.json instead. For the examples and the website, I've just moved everything to normal dependencies.

We now have a single package-lock.json containing all dependencies, so we can do npm ci on travis to save some time.

Best to make npm happy by removing all the node_modules folders first:

rm -r node_modules \
    website/node_modules \
    packages/uppy/node_modules \
    packages/@uppy/*/node_modules

The change list here is big because of all the lockfile changes. Almost every change to master will cause merge conflicts, but they are extremely easy to fix so we don't have to rush to get this in.

@goto-bus-stop goto-bus-stop requested a review from arturi July 15, 2019 12:04
package.json Show resolved Hide resolved
package.json Show resolved Hide resolved
@arturi arturi requested review from arturi and kvz July 15, 2019 14:18
@arturi
Copy link
Contributor

arturi commented Jul 16, 2019

We should update docs probably, everywhere we say to use bootstrap?

Copy link
Contributor

@arturi arturi left a comment

Choose a reason for hiding this comment

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

Very nice! Looking forward to not running bootstrap :) Checked dev example, website and some examples locally — all worked.

@goto-bus-stop
Copy link
Contributor Author

goto-bus-stop commented Jul 19, 2019

Ahh, think I figured out the CI failure finally.
When you npm install, npm tries to chmod +x any files that are mentioned in bin fields in package.json files. For Companion, that file was lib/standalone/start-server.js. But at the time the first npm install is done, we haven't been able to run build steps, so that file doesn't exist yet. I eventually fixed it by adding a bin/companion file that just require()s the file in lib/.

@goto-bus-stop goto-bus-stop merged commit 10bc795 into master Jul 19, 2019
@delete-merged-branch delete-merged-branch bot deleted the lerna-link-convert branch July 19, 2019 10:16
HeavenFox pushed a commit to docsend/uppy that referenced this pull request Jun 27, 2023
* lerna link convert

* ci: use npm ci

* update lockfile

* companion: set `bin` to source file

Since typescript doesn't actually transform anything, we can just use
this.

In a next major version we could set `noEmit: true` in the tsconfig and
stop publishing `lib`.

* companion: do chmod +x on start-server.js

* build: remove obsolete lerna config

* build: explicitly install latest versions when building e2e tests for ci

* Remove versions from private packages

* fix regex

* try fix

* ci: force npm to install to endtoend folder

* ci: fold up e2e build output

* Update netlify deploy preview command

* Remove mentions of npm run bootstrap

* Edit .github/CONTRIBUTING.md instead

* companion: add proxy executable

* companion: fix publish

* Downgrade jest to appease create-react-app
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