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

Bundler causes rejoining of original streams instead of modified ones #1335

Closed
tony19 opened this issue Jan 17, 2017 · 21 comments
Closed

Bundler causes rejoining of original streams instead of modified ones #1335

tony19 opened this issue Jan 17, 2017 · 21 comments

Comments

@tony19
Copy link
Contributor

tony19 commented Jan 17, 2017

Description

When the build stream is passed through the bundler, the result contains the original split streams instead of the modified ones (e.g., processed by uglify or other minifiers).

Versions & Environment

  • polymer-build: 0.6.0
  • node: 7.4.0
  • Operating System: macOS Sierra 10.12

Steps to Reproduce

  1. Generate project from generator-polymer-init-custom-build 2.0.0.

  2. Install gulp-uglify:

    yarn add -D gulp-uglify
    
  3. Edit the project's gulpfile.js to use gulp-uglify, and uncomment gulpfile.js:46, which runs uglify:

    const uglify = require('gulp-uglify');
    ...
    
    function build() {
      ...
      let sourcesStream = polymerProject.sources()
        .pipe(gulpif(/\.js$/, uglify()))
    
  4. Run gulp build.

  5. Observe the output of build/src/my-view1.html does not contain minified JS.

  6. Comment out gulpfile.js:69, which pipes the build through the bundler:

    buildStream = buildStream.pipe(polymerProject.bundler);
    
  7. Run gulp build.

  8. Observe the output of build/src/my-view1.html contains minified JS.

Expected Results

Output files contain modified streams.

For example, minified JS like this:

<script>Polymer({is:"my-view1"});let foo=1;console.log(foo++);</script>

Actual Results

Output files contain original/unmodified streams from HTMLSplitter.

For example:

<script>
  Polymer({
    is: 'my-view1',
  });

  let foo = 1;
  console.log(foo++);
</script>
@tony19
Copy link
Contributor Author

tony19 commented Jan 17, 2017

fyi, the polymer-bundler branch (from #108) didn't fix the bug in my tests :(

@davidsteinberger
Copy link

Just setting up a new project and ran into the exact same issue. Does anyone how to fix the bundler?

@FredKSchott
Copy link
Contributor

This was brought up in https://github.com/Polymer/polymer-build/issues/93 We are waiting on #108 to fix this since it moves us to the new bundler. Once that's in place, we can make sure it's getting the right files.

@tony19 link to the issue with the bugs that you're seeing?

@davidsteinberger
Copy link

@FredKSchott thanks for the explanation. Have u got a rough idea when #108 is going to land?

@FredKSchott
Copy link
Contributor

Both @usergenic and I are working on it all this week. If anyone would like to contribute a code review as well, feel free!

@davidsteinberger
Copy link

Is there any workaround until the new bundler lands? I tried crisper w/ vulcanize but that didn't workout too well, I guess b/c of the PRPL structure of the app.

@FredKSchott
Copy link
Contributor

Running optimizers after the bundler is the current work around.

@abdonrd
Copy link
Contributor

abdonrd commented Jan 31, 2017

I'm also having problems with this. Something new?

@abdonrd
Copy link
Contributor

abdonrd commented Feb 15, 2017

Friendly ping. :)

@FredKSchott
Copy link
Contributor

@usergenic has this on his radar but is juggling some other things as well. If anyone wants to take over and create a PR in the mean time I would be happy to help advise

@stramel
Copy link
Contributor

stramel commented Feb 18, 2017

Seems like v0.8.1 will work?

@FredKSchott
Copy link
Contributor

FredKSchott commented Feb 19, 2017

@stramel no fix for this has been released yet / in v0.8.1.

Again, until this is fixed the current work-around is to run optimizers after the bundler instead of before.

@web-padawan
Copy link
Contributor

BTW, what is the benefit of running optimizers before bundling? I mean, splitting and rejoining lot of files (e. g. 300+ elements) is slower than post-processing of few bundles, isn't it?

@FredKSchott
Copy link
Contributor

Optimizing before bundle lets you process your source files and dependencies separately. So if you wanted to use something like babel to compile your sources only, you can.

If you aren't handling your sources and dependencies separately, optimizing after bundling will probably give you a performance boost.

@stramel
Copy link
Contributor

stramel commented Feb 20, 2017

@FredKSchott Oops! You're correct, I think I got a bit over excited about the new release. Can't wait until this is fixed!

@myw
Copy link

myw commented Feb 22, 2017

@FredKSchott that is exactly my workflow: babel + bundle. The current state of things makes it impossible—since I can't not babel, my only workaround is to not bundle 😢 ;

In browsing through the source code, I think the issue is that this.analyzer.startAnalysis() gets called as soon as this.sources() or this.dependencies() are called. Those are necessarily called before this.bundler, because otherwise, you wouldn't know what you were bundling.

Whatever processing happens on the streams after this.dependencies() and this.sources() are called happens after the import resolution has already been determined. So when the bundler uses the analyzer to go through and figure out what to bundle, it's getting the stream that the analyzer computed before the streams were processed.

The old way of doing it had the analyzer as its own processor; the streams were piped through it after custom processing and after stream merge, immediately before bundling. It was a little slower, because you always had to analyze and then merge after all the post-processing was done: you couldn't do it asynchronously, like it currently works.

Unless you can somehow "plug" into a stream later, I don't see a great way around this—the least wrong thing I can come up with is a breaking change that returns to the "old way" of explicitly calling the analyzer after merging the streams back together.

@web-padawan
Copy link
Contributor

@myw you can use babel, why not? Just use following workflow: bundle - crisper - babel. See here and below my example (a bit outdated, since it doesn't use polymer-build yet, as I used to make bundles via web-component-shards previously).

@myw
Copy link

myw commented Feb 22, 2017

@web-padawan yes, that was the old workflow, even with polymer-build, somewhere around 0.4. I mistyped earlier: I meant to say, since I can't not babel

@tjmonsi
Copy link

tjmonsi commented Feb 23, 2017

@myw would it be possible to use two streams then? like the analyzer being used to analyze the original stream and then bundle the modified stream (like after babel). I would like to know where in the files where this works so I can look at it as well...

@myw
Copy link

myw commented Feb 23, 2017

@tjmonsi the problem is you can't get the stream without calling the analyzer

@stramel
Copy link
Contributor

stramel commented Mar 7, 2017

Is there going to be a tag soon for @usergenic's awesome fix?

@aomarks aomarks transferred this issue from Polymer/polymer-build Jan 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants