Remove use of momentjs #1652

lcampos · 2019-09-17T02:17:41Z

What does this PR do?

This PR replaces momentjs by using the native locale methods and other date formatting utilities. This is part of the changes needed to reduce the size of the extensions. By removing momentjs, we remove a dependency on a module that is not tree-shakeable.

What issues does this PR fix or reference?

@W-6587405@

lcampos · 2019-09-17T02:22:16Z

packages/salesforcedx-vscode-core/src/commands/forceApexLogGet.ts

-        'YYYYMMDDhhmmss'
-      );
+      const localUTCDate = new Date(response.data.startTime);
+      const date = getYYYYMMddHHmmssDateFormat(localUTCDate);


This is used in naming the debug log that gets downloaded when running SFDX: Get Debug Logs

lcampos · 2019-09-17T02:24:21Z

packages/salesforcedx-vscode-core/src/commands/forceApexLogGet.ts

@@ -135,16 +137,17 @@ export class LogFileSelector
    if (logInfos.length > 0) {
      const logItems = logInfos.map(logInfo => {
        const icon = '$(file-text) ';
+        const localUTCDate = new Date(logInfo.StartTime);
+        const localDateFormatted = localUTCDate.toLocaleDateString(


This is used for listing the available debug log records when running SFDX: Get Debug Logs

lcampos · 2019-09-17T02:26:46Z

packages/salesforcedx-vscode-core/src/decorators/traceflag-time-decorator.ts

@@ -5,7 +5,10 @@
 * For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
 */

-import * as moment from 'moment';
+import {


The changes on this file control the formatting of time and date showed in the footer text added after running SFDX: Turn On Apex Debug Log for Replay Debuger

codecov · 2019-09-17T02:33:41Z

Codecov Report

Merging #1652 into develop will increase coverage by 12.38%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##           develop    #1652       +/-   ##
============================================
+ Coverage    71.57%   83.95%   +12.38%     
============================================
  Files          190       68      -122     
  Lines         7373     3098     -4275     
  Branches       842      294      -548     
============================================
- Hits          5277     2601     -2676     
+ Misses        1920      439     -1481     
+ Partials       176       58      -118

Impacted Files	Coverage Δ
.../src/commands/templates/forceLightningAppCreate.ts
...s/salesforcedx-vscode-core/src/orgBrowser/index.ts
.../salesforcedx-vscode-core/src/diagnostics/index.ts
...ommands/templates/forceLightningInterfaceCreate.ts
...pex/src/languageClientUtils/languageClientUtils.ts
...orcedx-vscode-core/src/commands/templates/index.ts
...vscode-core/src/traceflag/developerLogTraceFlag.ts
...x-replay-debugger/test/integration/goldFileUtil.ts
...forcedx-vscode-core/src/commands/forceOrgCreate.ts
...-vscode-core/src/decorators/demo-mode-decorator.ts
... and 110 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 15b27df...e227a36. Read the comment docs.

sfsholden · 2019-09-17T14:21:04Z

packages/salesforcedx-utils-vscode/test/unit/date/format.test.ts

+    expect(doubleDigit).to.equal('09');
+  });
+
+  it('Should return a the same two character string when provided a double digit number', () => {


typo in test description

AnanyaJha

Changes look good! What made the momentjs module not be compatible with tree-shaking?

lcampos · 2019-09-17T16:33:54Z

Changes look good! What made the momentjs module not be compatible with tree-shaking?

@AnanyaJha this thread (moment/moment#2373) goes a bit more in depth into the challenges of breaking it into smaller pieces using tools like webpack but it boils down to the way the library is structured.

sfsholden · 2019-09-17T23:04:25Z

packages/salesforcedx-utils-vscode/test/unit/date/format.test.ts

+      optionYYYYMMddHHmmss
+    );
+
+    const dateTimeFormatRegex = /([0-9]{2})(\/)([0-9]{2})(\/)([0-9]{4})(\,)\s([0-9])(:)([0-9]{2})(:)([0-9]{2})\s(AM|PM)/;


@lcampos This test fails locally for me because the conversion from utc to mst causes the time stamp to be 10:34 instead of 9:34. The regex needs to be updated to:
([0-9]{2})(/)([0-9]{2})(/)([0-9]{4})(,)\s([0-9]{1,2})(:)([0-9]{2})(:)([0-9]{2})\s(AM|PM)

sfsholden · 2019-09-17T23:07:43Z

packages/salesforcedx-utils-vscode/test/unit/date/format.test.ts

+      optionHHmm
+    );
+
+    const timeFormatRegex = /([0-9])(:)([0-9]{2})\s(AM|PM)/;


If we update the last example, we should really update this one to ([0-9]{1,2})(:)([0-9]{2})\s(AM|PM). This one passes for me locally, but only because we beginning the regex at the start of the string.

Thanks for pointing this out, I'll open a follow up PR to take care of these failures.

Luis Campos added 2 commits September 16, 2019 17:57

Remove usage of momentjs, first part

d453a89

Move data format code to utils package and add tests

374592f

lcampos requested review from sfsholden, rcoringrato-sfdc, AnanyaJha, brpowell and cademay September 17, 2019 02:18

lcampos commented Sep 17, 2019

View reviewed changes

Update tests to work on any timezone

a55a51d

sfsholden reviewed Sep 17, 2019

View reviewed changes

Fix test typo

6128ebf

sfsholden approved these changes Sep 17, 2019

View reviewed changes

AnanyaJha approved these changes Sep 17, 2019

View reviewed changes

Merge branch 'develop' into lcampos/remove-momentjs

e227a36

lcampos merged commit 8a4af5f into develop Sep 17, 2019

lcampos deleted the lcampos/remove-momentjs branch September 17, 2019 17:06

sfsholden reviewed Sep 17, 2019

View reviewed changes

lcampos mentioned this pull request Sep 17, 2019

Fix regex for hour in time conversions #1663

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove use of momentjs #1652

Remove use of momentjs #1652

lcampos commented Sep 17, 2019 •

edited

lcampos Sep 17, 2019

lcampos Sep 17, 2019

lcampos Sep 17, 2019

codecov bot commented Sep 17, 2019 •

edited

sfsholden Sep 17, 2019

AnanyaJha left a comment

lcampos commented Sep 17, 2019

sfsholden Sep 17, 2019

sfsholden Sep 17, 2019

lcampos Sep 17, 2019

Remove use of momentjs #1652

Remove use of momentjs #1652

Conversation

lcampos commented Sep 17, 2019 • edited

What does this PR do?

What issues does this PR fix or reference?

lcampos Sep 17, 2019

Choose a reason for hiding this comment

lcampos Sep 17, 2019

Choose a reason for hiding this comment

lcampos Sep 17, 2019

Choose a reason for hiding this comment

codecov bot commented Sep 17, 2019 • edited

Codecov Report

sfsholden Sep 17, 2019

Choose a reason for hiding this comment

AnanyaJha left a comment

Choose a reason for hiding this comment

lcampos commented Sep 17, 2019

sfsholden Sep 17, 2019

Choose a reason for hiding this comment

sfsholden Sep 17, 2019

Choose a reason for hiding this comment

lcampos Sep 17, 2019

Choose a reason for hiding this comment

lcampos commented Sep 17, 2019 •

edited

codecov bot commented Sep 17, 2019 •

edited