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

feat(package): optimize bundle for package individually #87

Merged
merged 6 commits into from
Feb 26, 2021

Conversation

olup
Copy link
Contributor

@olup olup commented Feb 22, 2021

This PR allows for some optimization on the individual packaging. This is a WIP to get feedback from the main maintainer.

@olup
Copy link
Contributor Author

olup commented Feb 22, 2021

@floydspace I'd like your input on this - the flow is here but code could - should - probably be optimized.
Tests are also lacking atm. Plus serveless-offline will not work with the packaging, as aws-sdk is not bundled with it. The best way is surely to not package indivually when using offline.

@floydspace
Copy link
Owner

@olup thank you very much for the contribution, really appreciate, will check it during the day 👍

@olup
Copy link
Contributor Author

olup commented Feb 22, 2021

Actually, I am not too happy with this implementation. It works, but could be better. I think I can skip the copy / install / prune all together by only getting the list of required deps from the packager (from the intersection of externals and actually required dependency in the bundle) - and from there build the zip file by selecting what I allow inside. This would be very neat, I'll update the pr once I find a way to do that.

@floydspace
Copy link
Owner

@olup to be honest, I tested it and it didn't work for me. I had an issue with copying. Yes please take your time, it would be nice to have the fix. thank you

@olup olup force-pushed the feat/optimize-package-individually branch from af9de83 to 7325a78 Compare February 23, 2021 02:27
@olup
Copy link
Contributor Author

olup commented Feb 23, 2021

I just hard-pushed the branch with a way better system.
It works very well on one of my company's prod serverless repo, but more testing should be done. Some optimization is still required too. But this is a start.

@olup olup changed the title [WIP] feat(package): optimize bundle for package individually feat(package): optimize bundle for package individually Feb 25, 2021
@floydspace
Copy link
Owner

Hey @olup I tested, it worked perfectly for the case when we need to package externals.
Then I tested the case when we bundle everything, here I got an error Error: Cannot find module '/Users/victor/Projects/_own/individually/.build/package.json', so I fixed it by replacing buildDir with findProjectRoot() in line 54, so it works again
And also I fixed a bug with yarn packager.
Could you please check my commit from your perspective? Really appreciate your efforts. let me know if everything's fine I will merge the feature.

@olup
Copy link
Contributor Author

olup commented Feb 25, 2021

Ok, I saw the commit. Thank you for fixing the yarn bug ! However on the other thing : the no external case is indeed missing in my code, but I think we need to read the generated package.json, not the original, because it contains only references to the external, and have been generated after some logic already.
What we are doing here is to get the list of the externals of the complete project (in the generated package json) and reduce this list to only what appears in the actual bundle. So using the original package json will work too I guess, but I think it would be better to check for existence of the generated package.jdon, or to simply check if there are externals declared at all - and skip the whole part when it's not the case. I'll commit something in the morning and we can test, if you agree with what I said - but if I am being mistaken, please tell me.

@floydspace
Copy link
Owner

@olup yes go ahead, I agree, this logic should be skipped if we do not use externals

@olup
Copy link
Contributor Author

olup commented Feb 26, 2021

Just pushed. This makes the external part dependent on if there is externals or not. I also used the declaration of the externals as my comparison array. I wonder if it's ok (I mean, no use of regex and all in this, right ?).

This works with my test repo, with and without externals.

Also added bundle size in the logs, just because it's nice :-)

@floydspace
Copy link
Owner

@olup it works perfectly on my test case. Merging it

@floydspace floydspace merged commit b88bfc8 into floydspace:master Feb 26, 2021
@github-actions
Copy link

🎉 This PR is included in version 1.8.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

None yet

2 participants