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

Fix broken UTC plugin due to rollup #1453

Merged
merged 3 commits into from
May 26, 2021

Conversation

aloisklink
Copy link
Contributor

I was playing around with using dayjs from the dev branch with #1450 and Node.JS, and I noticed that the dayjs/plugin/utc was broken, as the plugin was instead exported under the default key.

> const dayjs = require("."); const utc = require("./plugin/utc"); dayjs.extend(utc);
Uncaught TypeError: t is not a function
    at Function.v.extend (/tmp/dayjs/dayjs.min.js:1:6319)
> console.log(utc);
{
  REGEX_VALID_OFFSET_FORMAT: /[+-]\d\d(?::?\d\d)?/g,
  REGEX_OFFSET_HOURS_MINUTES_FORMAT: /([+-]|\d\d)/g,
  default: [Function]
}

It looks like Rollup does not like mixing together named exports and default exports: https://rollupjs.org/guide/en/#default-export

Updating Rollup

I updated Rollup to the latest version, which fixed an error with running npm install --dev on NPM@v7, and also added the following warning:

me@me:/tmp/dayjs$ npm run build

> dayjs@0.0.0-development build
> cross-env BABEL_ENV=build node build && npm run size

Entry module "src/plugin/utc/index.js" is using named and default exports together. Consumers of your bundle will have to use `dayjs_plugin_utc["default"]` to access the default export, which may not be what you want. Use `output.exports: "named"` to disable this warning

> dayjs@0.0.0-development size
> size-limit && gzip-size dayjs.min.js


  Package size: 2.6 KB
  Size limit:   2.99 KB
  With all dependencies, minified and gzipped

2.86 kB

The new rollup is create a gzip file that is 31 bytes larger than the old gzip size. It seems like it's mainly due to the addition of globalThis, so this should also fix #1060, but I haven't tested it.

I also changed build/index.js to run rollup sequentially, since otherwise it uses up more than 16 GB of RAM on my computer, which makes it freeze.

Fixing plugins/utc

Finally, to fix plugins/utc, I moved the two regex named exports to the constant.js file. Normally this would be a breaking change, but since there hasn't been a release since the PR that added these exports (#1395), it should be fine.

New bundle sizes

Created by running for file in {plugin/*.js,dayjs.min.js}; do echo "$file: $(npx gzip-size --raw "$file")"; done
As can be seen, all the bundles have increased in size by about 15-30 bytes (mainly due to adding globalThis), except for the utc plugin, which decreases in size since it no longer has the extra exports.

I think it's still worth it though, since the old rollup version that is being used is from 2018-03-17, so it is quite out-of-date.

file old size new size
plugin/advancedFormat.js 583 602
plugin/arraySupport.js 315 337
plugin/badMutable.js 387 407
plugin/buddhistEra.js 342 361
plugin/calendar.js 405 427
plugin/customParseFormat.js 1697 1729
plugin/dayOfYear.js 242 263
plugin/devHelper.js 610 631
plugin/duration.js 1586 1616
plugin/isBetween.js 273 294
plugin/isLeapYear.js 207 230
plugin/isMoment.js 184 202
plugin/isoWeek.js 461 481
plugin/isoWeeksInYear.js 234 253
plugin/isSameOrAfter.js 195 215
plugin/isSameOrBefore.js 197 218
plugin/isToday.js 212 234
plugin/isTomorrow.js 226 247
plugin/isYesterday.js 229 249
plugin/localeData.js 683 705
plugin/localizedFormat.js 435 454
plugin/minMax.js 323 343
plugin/objectSupport.js 584 613
plugin/pluralGetSet.js 264 286
plugin/preParsePostFormat.js 396 414
plugin/quarterOfYear.js 384 403
plugin/relativeTime.js 718 737
plugin/timezone.js 1051 1069
plugin/toArray.js 201 224
plugin/toObject.js 230 250
plugin/updateLocale.js 230 251
plugin/utc.js 1022 967
plugin/weekday.js 251 269
plugin/weekOfYear.js 419 440
plugin/weekYear.js 231 251
dayjs.min.js 2829 2860

Update rollup to latest version to show new warnings.

As updating rollup-plugin-uglify does not support rollup@^2,
I've replaced it with rollup-plugin-terser, which is normally
the smallest: https://github.com/babel/minify#benchmarks

I also modified the build/index.js script to build
all of the locales/plugins sequentially, as I was reaching 100%
of my RAM usage, even on my 16 GB RAM PC.
It only save a few bytes in gzip-size, but still better than nothing.
The UTC plugin had both named exports and default exports
(export default; and export const example;)

Rollup.js states (see https://rollupjs.org/guide/en/#default-export):
It is bad practice to mix default and named exports in the same module,
though it is allowed by the specification.

Mixing both together means that for UMD builds, we can no longer
do `const utc = require("dayjs/plugin/utc")`, we must instead do
`const utc = require("dayjs/plugin/utc").default`,
which would be a breaking change.

Instead, I've just placed the named exports in the constant.js file
instead. This has the benefit of reducing the gzip-size of plugin/utc
from 1053 to 967 bytes as well.
@aloisklink aloisklink mentioned this pull request Apr 23, 2021
@iamkun
Copy link
Owner

iamkun commented May 26, 2021

LGTM

@iamkun iamkun merged commit 0914706 into iamkun:dev May 26, 2021
@iamkun
Copy link
Owner

iamkun commented May 26, 2021

🎉 This PR is included in version 1.10.5 🎉

The release is available on:

Your semantic-release bot 📦🚀

@aloisklink aloisklink deleted the fix-rollup-utc-plugin branch May 27, 2021 15:26
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.

umd package in esmodule 'this' error
3 participants