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

modular build <library> #195

Merged
merged 9 commits into from
Dec 23, 2020
Merged

modular build <library> #195

merged 9 commits into from
Dec 23, 2020

Conversation

threepointone
Copy link
Contributor

@threepointone threepointone commented Dec 18, 2020

This PR adds a feature to let modular build workspaces as libraries (as opposed to only apps). It uses rollup to generate commonjs and esm builds of the workspace it's pointing to (based on the main field in the workspace's package.json), and then generates typescript typings for the same. It then generates a publishable folder under dist/<workspace name>, with a modified package.json that points to these built versions of the files. The folder will also contain any other files specified by the files field, and/or .npmignore in the workspace. Passing --preserve-modules decides whether or not to make a bundle.

You can also do parallel builds by passing a comma-separated list of packages: modular build app1,app2,package1,view1,etc

Finally, I made it so app builds go to /dist/<app-name>, for consistency.

This is a port of the script we used internally at JPM for the same, and makes it a first class feature for modular. It's fast and works fairly well.

Three things missing from this:

  • We should use it for our own builds
  • No tests for parallel/multiple builds
  • Docs

I'll do these in the next PR.

threepointone and others added 2 commits December 11, 2020 19:35
…as opposed to apps). It uses rollup to generate commonjs and esm builds of the workspace it's pointing to (based on the `main` field in the workspace's `package.json`), and then generates typescript typings for the same. It then generates a publishable folder under `dist/<workspace name>`, with a modified `package.json` that points to these built versions of the files. The folder will also contain any other files specified by the `files` field, and/or `.npmignore` in the workspace.

This is a port of the script we used internally at JPM for the same, and makes it a first class feature for modular. It's fast and works fairly well.
@changeset-bot
Copy link

changeset-bot bot commented Dec 18, 2020

🦋 Changeset detected

Latest commit: 282a106

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
modular-scripts Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

package.json Show resolved Hide resolved
packages/modular-scripts/src/build.ts Outdated Show resolved Hide resolved
@@ -12,7 +12,7 @@ import {
} from 'change-case';
import prompts from 'prompts';
import resolveAsBin from 'resolve-as-bin';

import buildPackage from './build';
Copy link
Contributor

Choose a reason for hiding this comment

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

Build does a lot of top level work - so potentially we want to hide this import or change the top level work done?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't understand this comment, could you clarify what you mean? Do you want me to move build() into this file?

Copy link
Contributor

Choose a reason for hiding this comment

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

My main concern is that there are import side-effects for build.ts e.g. [1] / [2] which do a lot of FS operations. On a machine with a slow-FS (e.g. network drives) this could slow down the startup of the cli for all operations. I think we should either:

  1. Remove the globals from the build.ts script and have them as locals for the build operations, or.
  2. Lazily import build.ts so that the initialisation only happens when it's needed.

[1] https://github.com/jpmorganchase/modular/pull/195/files#diff-a3c185d299c664d2bc8bbc2bbade48ca2a96a5ed03c585d268e5290e97016c0aR77
[2] https://github.com/jpmorganchase/modular/pull/195/files#diff-a3c185d299c664d2bc8bbc2bbade48ca2a96a5ed03c585d268e5290e97016c0aR96

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good points. Ok lemme rewrite this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok this import is now deferred till the time it's actually called.

packages/modular-scripts/src/build.ts Outdated Show resolved Hide resolved
This was referenced Dec 20, 2020
@LukeSheard LukeSheard self-requested a review December 23, 2020 14:17
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