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

Run optimized dependency install after build #526

Merged
merged 3 commits into from
Jun 23, 2020

Conversation

FredKSchott
Copy link
Owner

@FredKSchott FredKSchott commented Jun 20, 2020

Resolves #475, based on #525

This PR solves all of the tree-shaking issues we've seen since adding it back into Snowpack, mainly around import {TSType} and some of the more custom compile-to-JS languages like Svelte & Vue where the compiler adds imports that our current source code scanner never sees.

This replaces our current build workflow:

  1. Install dependencies into a temporary directory (tree-shaking based off the incomplete source code)
  2. Copy temp dependencies directory into your build directory
  3. Build your source files, resolve dependency imports, and write to disk

With a new, more explicit workflow:

  1. Built your source files (not yet resolving imports or saving to disk)
  2. Pass these in-memory built files to our installer for tree-shaking analysis
  3. Install dependencies directly into build, with tree-shaking based off of complete built source code
  4. Now that we have installed dependencies, resolve your imports.
  5. Save files to disk.

As a happy bonus, running install as a part of the build means that its output isn't immediately hidden by the first paint of dashboard. Install logs now show up in the Console portion of the dashboard. As a follow-up, we should make this its own section (▼ Install Optimized Dependencies).

Somewhat related: this feels like the final nail in the coffin for representing "where will we install web_modules?" inside of a "mount" script. Now that we have replaced our old "install-then-copy" with an "install directly into build dir", "mounting" this already-fake directory makes even less sense. I think it's time to treat "web_modules" as the special concept that it is, and give it its own "buildOptions.webModulesPath" configuration.

@FredKSchott FredKSchott requested a review from drwpow June 20, 2020 19:23
@FredKSchott FredKSchott requested a review from a team as a code owner June 20, 2020 19:23
@vercel
Copy link

vercel bot commented Jun 20, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/pikapkg/snowpack/q1me3z3dy
✅ Preview: https://snowpack-git-optimize-build-install.pikapkg.vercel.app

src/commands/build.ts Outdated Show resolved Hide resolved
src/commands/build.ts Outdated Show resolved Hide resolved
src/config.ts Outdated Show resolved Hide resolved
);
// 3. Print stats immediate after install output.
if (installResult.stats) {
console.log(printStats(installResult.stats));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could this printStats() live in the install runner? Or are there times when we don’t want the install command to write to stdout?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Yup, this is in preparation for some of the work happening in #523, and a time when we'll be able to configure install() to not write to stdout/stderr, if you want to call it programatically.

Copy link
Collaborator

@drwpow drwpow left a comment

Choose a reason for hiding this comment

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

This makes sense to me! I had some minor questions / comments but nothing blocking.

Base automatically changed from programmatic-install to master June 23, 2020 00:11
@FredKSchott FredKSchott merged commit 6c4e12a into master Jun 23, 2020
@FredKSchott FredKSchott deleted the optimize-build-install branch June 23, 2020 16:54
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.

Tree-shake JS using build result, instead of source
2 participants