Update dependencies #578
Update dependencies #578
Conversation
The default location on macOS contains spaces (Application Support)
When using defineProperties, any properties that are set default to writable: false unless you specify otherwise. Moving the assignment before the defineProperties call fixes the issue and leaves it ambiguous as to whether the property is writable or not.
We'll tackle this later to avoid a noisy PR
Will slowly start to transition to checking with ESLint since TSLint is deprecated
Ignores any failures for now
This has become a linting concern instead of a compiler concern. To get the same check to ensure we use the `type` import, added an ESLint rule that checks. No TSLint rule exists that covered this case.
Not sure if/when this changed, but as of right now this matches the polyfill source found here: https://github.com/js-temporal/temporal-polyfill/blob/a5eda13f7ce9d2f2df899c2ac56e92936ec8d1bf/lib/ecmascript.ts#L2679
| // Svelte doesn't correctly compile if imports of the generated /modules | ||
| // aren't imported as 'import type' in other parts of the generated | ||
| // querybuilder, so set this option to ensure we always do that | ||
| "@typescript-eslint/consistent-type-imports": "error" |
There was a problem hiding this comment.
This ESLint rule replaces the now deprecated tsconfig flag "importsNotUsedAsValues": "error". See the PR introducing the change to TypeScript and the section in the release announcement.
TL;DR: The intention of the importsNotUsedAsValues flag was never really to be a linting rule to disallow importing a type without the import type syntax. It also has weird interactions with isolatedModules and preserveValueImports which had some drastic effects on the transpiled JavaScript.
| - name: ESLint (ignore failures for now) | ||
| run: | | ||
| yarn eslint || echo "ESLint still failing... Fine for now!" | ||
|
|
There was a problem hiding this comment.
Making this an always passing step in the CI for now, and plan on tackling issues as I spot them in files that we touch over the next few months so we can switch over fully away from TSLint.
| this.year < 0 || this.year > 9999 | ||
| ? (this.year < 0 ? "-" : "+") + | ||
| Math.abs(this.year).toString().padStart(6, "0") | ||
| : this.year.toString(); | ||
| : this.year.toString().padStart(4, "0"); |
There was a problem hiding this comment.
This was failing, but it doesn't seem related to any changes I made here. I'm not sure why this is now breaking, but perhaps it was always flaky in tests since it is randomly generated? At any rate, here is the same bit of code from the polyfill we use:
if (year < 0 || year > 9999) {
const sign = year < 0 ? '-' : '+';
const yearNumber = MathAbs(year);
yearString = sign + `000000${yearNumber}`.slice(-6);
} else {
yearString = `0000${year}`.slice(-4);
}I think this update matches that logic precisely.
There was a problem hiding this comment.
Looks like the polyfill version got updated from 0.4.2 to 0.4.3 in the lockfile, and the new version contains this change. Eventually when the Temporal api is finalised and makes its way into browsers/node/etc. we'll be able to throw all these custom datetime types away 🎉.
There was a problem hiding this comment.
Ahh, of course I should've checked the yarn.lock file not just the package.json 🤦 .
Eventually when the Temporal api is finalised and makes its way into browsers/node/etc. we'll be able to throw all these custom datetime types away 🎉.
Yeah, what has been our policy for runtime support in the past? If something is so easily polyfilled like this, it feels like it's fine to put that on the consumer to polyfill, yeah? I guess the same question could be said for our fetch polyfill, so maybe we can just make some kind of executive decision about it.
| // "target": "es2017", | ||
| // "strictPropertyInitialization": false, | ||
| // "baseUrl": ".", |
There was a problem hiding this comment.
I'm not a big fan of commented out code, so I just removed all this extra stuff 🤷
Going to be adding a bunch of performance benchmarking and I want to start on a clean slate with TypeScript 5.0+ and thought it was a good time to do a little housekeeping on the dev dependencies.
Some things to note: