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

excludes in JS minify being skipped #279

Open
Westbrook opened this issue May 1, 2018 · 12 comments
Open

excludes in JS minify being skipped #279

Westbrook opened this issue May 1, 2018 · 12 comments
Labels

Comments

@Westbrook
Copy link

I've get a situation very close to https://github.com/Polymer/polymer-cli/issues/701 and while I've applied the approach added via Polymer/polymer-cli#952 I'm noticing that the files in question are still being minified in a way that triggers errors in the Firebase code.

Tracking through the build/cli code I noticed that when the files are entering the getOptimizeStreams section of the build pipeline, the file was being tracked with a rewritten file.pathName such that file. relative would return an unmatchable file name when passed to notExcluded for comparison. The can be experienced via https://github.com/Westbrook/polymer-build-filenames which outlines the steps to see this in action.

Upon further review with some help from @justinfagnani I realized that all the the JS files, no matter their origin, were being seen as isInline === true at https://github.com/Polymer/tools/blob/master/packages/build/src/html-splitter.ts#L189 which would clearly cause this issue. It seems be happening because bundle is occurring before getOptimizeStreams here https://github.com/Polymer/tools/blob/master/packages/cli/src/build/build.ts#L84 It seems that this issue can be avoided by moving the if (bundled) { block to just before the if (options.basePath) { block here https://github.com/Polymer/tools/blob/master/packages/cli/src/build/build.ts#L128 and wanted to see if that made sense to someone more knowledgable on this project that I.

I'd be happy to PR this if you'd like, though I know there is lots of dev going against this right now. Either way let me know if I can help get the right fix for this into the codebase!

@davie-robertson
Copy link

I thought I was going mad - I've been facing the same issue for weeks.

https://stackoverflow.com/questions/50073467/

Justin - I think this is the same issue (or at least related) I asked you to take a look at (via twitter) regarding a lib (UnityLoader.js) that keeps getting screwed with in the build process (even taking the code from my custom element and incorporating in directly in my App now breaks during an ES5 build)

@Westbrook
Copy link
Author

FYI, I'm still experiencing this issue with a fresh install of v1.7.3. I'd be happy to submit a PR if the code ordering was seen as an acceptable fix for this issue. Please advice.

@iSuslov
Copy link

iSuslov commented Aug 24, 2018

Still the same in 1.8.0 and issue submitted in "old" repository. https://github.com/Polymer/polymer-cli/issues/1008
Poor documentation, and after one tries multiple approaches wasting time it turns out that it is simply not working.

So for anyone who needs excludes for js to work, use v1.6.0

@christophe-g
Copy link
Contributor

Gentle ping.

Seems to be an important issue (... at least preventing me to upgrade).

@christophe-g
Copy link
Contributor

@usergenic - any chance for the cli to honor minify.exclude as it used to ?

@christophe-g
Copy link
Contributor

@usergenic - not giving up ; )

@DrNiels
Copy link

DrNiels commented May 22, 2019

Exclusion still doesn't seem to work with 1.9.9. What is the status here? Is there some workaround for the time being? Or would I need to manually copy the excluded dependencies after build?

@Westbrook
Copy link
Author

FYI: while I was able to get by with the code suggested in #584 I was never able to get the tests to pass and never did the work upgrade the pull to the current version of the CLI. I moved away from using polymerfire to rxfire for the project relying on this feature and have had good success. Particularly, polymerfire being a bit of a blocker to moving to Polymer 3/LitElement I was very happy with the flexibility I achieved with the change. I realize there are other libraries that might suffer from this issue, and in those cases (if you're on Polymer 3/LitElement already) it might be better to use Rollup/Webpack via the suggestions found here: https://open-wc.org/building/#using-index-html-as-an-entry-point

@usergenic
Copy link
Contributor

Hey everyone, thanks for your patience. I'm working on getting this in right now.

@usergenic
Copy link
Contributor

Okay so one issue with the PR moving the bundler to after the JS transformation stuff happens is that bundler doesn't know how to bundle anything which has been transformed to AMD.

While I investigate refactoring things, you might try adding the files you want to exclude to the config's bundle > excludes array as well as the js > minify > exclude array (and don't ask why one term is plural and one is singular) in your polymer.json:

{
  "npm": true,
  "moduleResolution": "node",
  "build": [
    {
      "name": "bundled-es",
      "bundle": {
        "excludes": [ "./path/to/whatever.js" ]
      },
      "js": {
        "minify": {
          "exclude": [ "./path/to/whatever.js" ]
        }
      }
    }
  ]
}

@usergenic
Copy link
Contributor

usergenic commented May 24, 2019

The reason why the js.minify.excludes bit isn't working for you right now (it's not "broken" per se) is that when you're bundling, the bundles have already been constructed by the time the filenames are matched in the minifier against exclusion list. You can exclude the specific bundle from minification if you want if you know that bundles name and that should work (if not then it is a bug)

@stale
Copy link

stale bot commented May 25, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

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

No branches or pull requests

6 participants