-
Notifications
You must be signed in to change notification settings - Fork 158
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
Update build dependencies #690
Conversation
Notable updates: - prettier->v2 - rollup->v2 (sped up builds by a few seconds) - replaced deprecated rollup plugins - replaced uglify with terser for rollup 2.x - added --cache option to lint step (5x faster) - moved dev dependencies into dev section in package.json
Also: allow @ts* directives which let me verify test code with TSC
- Explicitly added `babelHelpers` option to avoid warning message - Removed invalid `lib` option - Switched unglify->terser which has a supported rollup V2 plugin
Codecov Report
@@ Coverage Diff @@
## main #690 +/- ##
==========================================
- Coverage 94.41% 94.41% -0.01%
==========================================
Files 17 17
Lines 4780 4778 -2
Branches 757 757
==========================================
- Hits 4513 4511 -2
Misses 264 264
Partials 3 3
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
@@ -8,27 +8,27 @@ | |||
}, | |||
"main": "polyfill/lib/index.mjs", | |||
"dependencies": { | |||
"ecmarkup": "^3.22.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The split between dependencies and devDependencies is a bit weird for the root package anyway. The package that's published to npm is only the polyfill/
directory, and for the rest (spec, documentation, meeting minutes) there really are no runtime dependencies as such.
return Temporal.DateTime.from(dateTime) | ||
.inTimeZone(timeZone, { disambiguation: 'reject' }) | ||
.inTimeZone(localTimeZone); | ||
return Temporal.DateTime.from(dateTime).inTimeZone(timeZone, { disambiguation: 'reject' }).inTimeZone(localTimeZone); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Glad prettier changed their minds on this!
import builtins from "rollup-plugin-node-builtins"; | ||
import resolve from "@rollup/plugin-node-resolve"; | ||
import {uglify} from 'rollup-plugin-uglify'; | ||
import { terser } from 'rollup-plugin-terser'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I looked into switching to terser some time ago and found it didn't work. But now I can't remember why. It seems to work now, so maybe the problem was with rollup v1.x. Let's go ahead with this, we can always revert if it causes problems.
While troubleshooting #687, I ended up updating all build dependencies except demitasse. Updating didn't fix the problem but all builds and tests still worked fine, so while I was in the neighborhood I figured I'd turn that work into a PR instead of rolling back. Here's what's in it:
--cache
option to lint step, which makes linting ~5x faster after the first run