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

fix(packaging): exclude optional peer dependencies #113

Merged
merged 6 commits into from Apr 15, 2021
Merged

Conversation

olup
Copy link
Contributor

@olup olup commented Apr 13, 2021

This excludes optional dependencies to be installed when resolving peer dependencies.

It also replaces glob with globby in pack.ts, however, this does not change much anything as we still need to stat for the file mode. Globby can stat for us in one pass, but the types are not yet complete on their side. I took the lazy route here and just replaced glob. We can improve that later on.

// this is needed for dependencies that maps to a path (like scoped ones)
!depWhiteList.find(dep => doSharePath(localPath, 'node_modules/' + dep))
)
const filesPathList = files
Copy link
Contributor Author

Choose a reason for hiding this comment

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

from there on it's just auto-formatting from my end. Is my formatting wrong or is it Calle Kabo's version that is not synced with the prettierrc ?

@olup
Copy link
Contributor Author

olup commented Apr 14, 2021

Added the exclusions of peer dependencies when listed in the exclude option

@olup olup linked an issue Apr 14, 2021 that may be closed by this pull request
src/pack-externals.ts Outdated Show resolved Hide resolved
src/pack.ts Outdated Show resolved Hide resolved
src/pack.ts Outdated Show resolved Hide resolved
@olup olup requested a review from floydspace April 15, 2021 10:48
@arpadgabor
Copy link

@olup I've tested the latest update to this PR locally and it's back to square one.

Serverless: Packing external modules: knex@^0.95.4, mysql, mysql2, pg@^8.6.0, pg-native, sqlite3, tedious
 
 Error ---------------------------------------------------
 
  Error: npm install failed with code 1
      at ChildProcess.<anonymous> (/home/arpad/serverless-esbuild-test/node_modules/serverless-esbuild/dist/utils.js:48:24)
      at ChildProcess.emit (events.js:314:20)
      at ChildProcess.EventEmitter.emit (domain.js:483:12)
      at maybeClose (internal/child_process.js:1022:16)
      at Process.ChildProcess._handle.onexit (internal/child_process.js:287:5)
external: ['knex', 'pg'],
packager: 'npm' // same with yarn

@olup
Copy link
Contributor Author

olup commented Apr 15, 2021

@arpadgabor Yep, caught that, fixed now. This should be running now and the PR should be complete.
Edit : tested on the test repo - seems to work fine, package gets done, and ms is indeed included in the package.json.

Copy link
Owner

@floydspace floydspace 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 @olup

@floydspace floydspace merged commit 753e4ae into master Apr 15, 2021
@github-actions
Copy link

🎉 This PR is included in version 1.10.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

Any way to use knex? Prevent installation of optional peer dependencies when packaging externals
3 participants