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

Minification issue with flow-router #11756

Closed
brianlukoff opened this issue Nov 12, 2021 · 12 comments · Fixed by #11758
Closed

Minification issue with flow-router #11756

brianlukoff opened this issue Nov 12, 2021 · 12 comments · Fixed by #11758

Comments

@brianlukoff
Copy link
Contributor

At some point post-2.3.4, the minification process of JS files seems to have broken in some way. To reproduce, create a plain app and add kadira:flow-router:

meteor create repro
cd repro
meteor add kadira:flow-router
meteor run --production

Then look at the minified source code:

Screen Shot 2021-11-11 at 3 40 46 PM

Compare this to the actual function -- the minification seems to have dropped the last part of doRedirect!

https://github.com/kadirahq/flow-router/blob/master/client/triggers.js#L91

@brianlukoff
Copy link
Contributor Author

I think it's a bug in terser's evaluate option (possibly terser/terser#837) and that we can avoid by adding evaluate: false to the terser compress options -- I've created this pull request: #11758

@filipenevola
Copy link
Collaborator

Published minifier-js@2.7.2.

@brianlukoff
Copy link
Contributor Author

I'm having trouble getting this updated version to be used by Meteor. My versions file is now showing 2.7.2 in use, but it seems to be using an older version when actually minifying the code (since run into the same problem that started this issue). But if I add standard-minifier-js to my app's packages directory (i.e., from a checkout), then Meteor does seem to use 2.7.2 and minification works as expected. I'm not sure what I am missing here?

@filipenevola
Copy link
Collaborator

@brianlukoff I published the standard-minifier-js now as well.

Could you check if this solves the problem?

@brianlukoff
Copy link
Contributor Author

Thanks! I'm not able to update to 2.7.2 though -- getting the following when I try to put @2.7.2 in packages:

While selecting package versions:
error: No version of standard-minifier-js satisfies all constraints: @2.7.2, @=2.7.1, @~2.7.1
Constraints on package "standard-minifier-js":
* standard-minifier-js@2.7.2 <- top level
* standard-minifier-js@=2.7.1 <- top level
* standard-minifier-js@~2.7.1 <- top level

@filipenevola
Copy link
Collaborator

@brianlukoff maybe you still have the local copy?

Or are you running from a checkout without pulling devel?

@brianlukoff
Copy link
Contributor Author

Oops -- you are right -- forgot to remove my local copy. Using standard-minifier-js@2.7.2 fixed it -- thanks!

@filipenevola
Copy link
Collaborator

We need to find another fix for this problem.

As I had to revert this fix in 2.7.3.

#11775 (comment)

@afrokick
Copy link
Contributor

afrokick commented Nov 25, 2021

@brianlukoff There is recommended flow-router fork created by @dr-dimitru

https://github.com/VeliovGroup/flow-router

meteor add ostrio:flow-router-extra

It is more up-to-date with regular release cycle. Could you check it?

@filipenevola
Copy link
Collaborator

Yes, this is what I suggested here as well.

@brianlukoff
Copy link
Contributor Author

@afrokick Thanks -- I'll definitely look into switching, but see my comment in this thread as to why I'm still concerned about the behavior of terser.

@filipenevola
Copy link
Collaborator

We are deprecating kadira:flow-router in favor of ostrio:flow-router-extra.

This issue only affects the old flow-router due to old JS patterns.

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

Successfully merging a pull request may close this issue.

4 participants