metron-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "ASF GitHub Bot (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (METRON-1796) [UI] Migrate off moment.js
Date Wed, 03 Oct 2018 13:28:00 GMT

    [ https://issues.apache.org/jira/browse/METRON-1796?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16636946#comment-16636946
] 

ASF GitHub Bot commented on METRON-1796:
----------------------------------------

GitHub user ruffle1986 opened a pull request:

    https://github.com/apache/metron/pull/1219

    METRON-1796: [UI] Migrate off moment.js

    ## Contributor Comments
    
    [DISCUSS] thread on the dev mailing list: https://lists.apache.org/thread.html/2e4fafa4256ce14ebcd4433420974e24962884204418ade51f0e3bfb@%3Cdev.metron.apache.org%3E
    
    Original ASF Jira ticket: https://issues.apache.org/jira/browse/METRON-1796
    
    The purpose of this PR is the complete removal of [moment.js](http://momentjs.com/) from
the code base and replacing it with either native functions or [date-fns](https://date-fns.org/),
another date manipulation library.
    
    **Motivation**
    
    In the Metron Alerts UI, we use a few functions only from moment.js to deal with time
like formatting or displaying the relative time from "now". In order to do that, we have to
import the entire library which causes a significant footprint in the compiled production
bundle. Sometimes they can be replaced with native built-in solutions, sometimes they cannot
but we can use date-fns which has a smaller size comparing to moment.js
    
    As of date-fns v2 is in alpha phase, I rather chose the latest stable version which is
1.29.0. Tree-shaking is currently not supported in version 1 but [it will be in version 2](https://date-fns.org/v2.0.0-alpha.9/docs/ECMAScript-Modules).
So once it's released and we can upgrade to it, we can see more size loss in the bundle size.
Angular uses Webpack under the hood so if you're interested in what tree-shaking is and how
Webpack does it, [follow this link](https://webpack.js.org/guides/tree-shaking/) for further
details.
    
    **Changes included**
    
    - Every code used moment.js is replaced with a built-in function or the appropriate function
from date-fns.
    
    I highly recommend you to go through all the commits I've made one by one. They're straightforward
and easier to understand which parts of the app are affected and in what way.
    
    **Testing the bundle file size**
    
    1. Checkout `master`
    1. Go to `metron-interface/metron-alerts`
    1. Make sure you have the latest packages in the `node_modules` folder via `npm ci`
    1. Run `npm run build`
    1. Take a look at the size of the `main.js` file
    1. Checkout `ruffle1986/METRON-1796`
    1. Run `npm ci` (this will install the date-fns library and remove moment.js)
    1. Run `npm run build`
    1. Take a look at the size of the `main.js` file and compare it to the number you've got
in step 4
    
    Here's the output of `npm run build` on the `master` branch:
    ![screen shot 2018-10-03 at 13 22 29](https://user-images.githubusercontent.com/2196208/46411822-08c66980-c71d-11e8-964b-6884c69b9d28.png)
    
    And here's the output on this branch:
    ![screen shot 2018-10-03 at 13 28 08](https://user-images.githubusercontent.com/2196208/46411889-2d224600-c71d-11e8-9ad9-fb0fbd58807a.png)
    
    As you can see, by removing moment.js, we could reduce the bundle size to 1.65 MB which
is **15.6%** comparing to the bundle with moment.js. Not so significant but still.
    
    You've probably noticed the warning message on the second picture below the output of
the build process. This is the result of compiling `pikaday-time` into our bundle via Angular.
For the record, it's totally fine. Moment.js is just an optional dependency of [Pikaday](https://dbushell.com/Pikaday/),
it works perfectly fine without it. This warning message is there because Pikaday [checks
whether it's a Node.js environment and since it is, it wants to load moment from the node_modules
folder](https://github.com/dbushell/Pikaday/blob/master/pikaday.js#L12-L16). However Pikaday
doesn't throw and fails silently, the Angular compiler is clever enough to catch this and
kindly warn you about it. But it doesn't cause any negative effect in the Metron UI because
(again) moment.js is not a required dependency of Pikaday.
    
    ## Pull Request Checklist
    
    Thank you for submitting a contribution to Apache Metron.  
    Please refer to our [Development Guidelines](https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=61332235)
for the complete guide to follow for contributions.  
    Please refer also to our [Build Verification Guidelines](https://cwiki.apache.org/confluence/display/METRON/Verifying+Builds?show-miniview)
for complete smoke testing guides.  
    
    
    In order to streamline the review of the contribution we ask you follow these guidelines
and ask you to double check the following:
    
    ### For all changes:
    - [X] Is there a JIRA ticket associated with this PR? If not one needs to be created at
[Metron Jira](https://issues.apache.org/jira/browse/METRON/?selectedTab=com.atlassian.jira.jira-projects-plugin:summary-panel).
    - [X] Does your PR title start with METRON-XXXX where XXXX is the JIRA number you are
trying to resolve? Pay particular attention to the hyphen "-" character.
    - [X] Has your PR been rebased against the latest commit within the target branch (typically
master)?
    
    
    ### For code changes:
    - [X] Have you included steps to reproduce the behavior or problem that is being changed
or addressed?
    - [X] Have you included steps or a guide to how the change may be verified and tested
manually?
    - [X] Have you ensured that the full suite of tests and checks have been executed in the
root metron folder via:
      ```
      mvn -q clean integration-test install && dev-utilities/build-utils/verify_licenses.sh

      ```
    
    - [X] Have you written or updated unit tests and or integration tests to verify your changes?
    - [X] If adding new dependencies to the code, are these dependencies licensed in a way
that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
    - [X] Have you verified the basic functionality of the build by building and running locally
with Vagrant full-dev environment or the equivalent?
    
    #### Note:
    Please ensure that once the PR is submitted, you check travis-ci for build issues and
submit an update to your PR as soon as possible.
    It is also recommended that [travis-ci](https://travis-ci.org) is set up for your personal
repository such that your branches are built there before submitting a pull request.


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/ruffle1986/metron METRON-1796

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/metron/pull/1219.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #1219
    
----
commit 0848d6f777e987e1b91cda535594bd7da5d5c216
Author: ruffle1986 <ftamas.mail@...>
Date:   2018-09-28T09:25:55Z

    cover timeRangeToDisplayStr with unit tests

commit 039edf4e3d00338280caea2e03267d626eff28da
Author: ruffle1986 <ftamas.mail@...>
Date:   2018-09-28T09:26:25Z

    add date-fns as dependency

commit 47684ced0bb932e7de43433fa3bb9fa04fa198d1
Author: ruffle1986 <ftamas.mail@...>
Date:   2018-09-28T09:31:10Z

    replace moment with date-fns in timeRangeToDisplayStr

commit 095600ea2204707b918899c9b782d4c30c6f9eac
Author: ruffle1986 <ftamas.mail@...>
Date:   2018-09-28T11:22:26Z

    refactor(alert-details): use date-fns to calculate fromNow

commit fefeb7883fb3b35bad005e751a0c53e8cc5215be
Author: ruffle1986 <ftamas.mail@...>
Date:   2018-09-28T11:30:33Z

    refactor(pcap-filters): use date-fns to format date

commit 2a077fc65c499c2acd16258d1d7920d805189b49
Author: ruffle1986 <ftamas.mail@...>
Date:   2018-09-28T11:57:55Z

    refactor(date-picker): stop using moment.js

commit 716e0960f9f43257c32b6d9337ff17321c93f38f
Author: ruffle1986 <ftamas.mail@...>
Date:   2018-09-28T12:02:40Z

    refactor(pcap-filters): remove moment.js and use date-fns in test

commit d95230c3de14417b868752b1fd6b4ac722d5468b
Author: ruffle1986 <ftamas.mail@...>
Date:   2018-09-28T12:14:46Z

    refactor(time-lapse.pipe) remove moment.js and use date-fns

commit 1cff5107dd2fe5bc3097f149537f8a2f3fc92d81
Author: ruffle1986 <ftamas.mail@...>
Date:   2018-09-28T12:26:30Z

    refactor(time-range): replace moment.js with date-fns (format)

commit 6f6fd6e67691591d32bf9d92cceff1407c8775f5
Author: ruffle1986 <ftamas.mail@...>
Date:   2018-10-01T14:10:19Z

    chore(utils.spec): add license header

commit b57f535b363eca6cf595f01df819dac5e1a654aa
Author: ruffle1986 <ftamas.mail@...>
Date:   2018-10-03T10:34:54Z

    test(alert-list): remove moment from e2e test

commit 2863708b54690570e76fdc6f261a7e647802cbe0
Author: ruffle1986 <ftamas.mail@...>
Date:   2018-10-03T10:35:44Z

    chore: remove moment from package.json and add patched pikaday-time

commit e5d0fab6ba55edf400976b56f0e9fcc0029491cd
Author: ruffle1986 <ftamas.mail@...>
Date:   2018-10-03T10:36:25Z

    refactor(date-picker): import patched pikaday-time

----


> [UI] Migrate off moment.js
> --------------------------
>
>                 Key: METRON-1796
>                 URL: https://issues.apache.org/jira/browse/METRON-1796
>             Project: Metron
>          Issue Type: Improvement
>            Reporter: Tamas Fodor
>            Assignee: Tamas Fodor
>            Priority: Minor
>
> Remove Moment.js and replace with another smaller library.
> Moment.js requires us to import the entire library vs. a few necessary modules.
> Moment.js can prevent bundlers from supporting tree-shaking.
> By removing Moment.js, we can decrease our overall bundle size and prevent issues with
tree-shaking in the future.
> Here you can find the discussion on the mailing list:
> https://lists.apache.org/thread.html/2e4fafa4256ce14ebcd4433420974e24962884204418ade51f0e3bfb@%3Cdev.metron.apache.org%3E



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Mime
View raw message