-
Notifications
You must be signed in to change notification settings - Fork 67
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 to ESLint 8 & Supported TypeScript to >4.5.3 #2216
Update to ESLint 8 & Supported TypeScript to >4.5.3 #2216
Conversation
… for node 14.17 and below
…te to rollup-plugin-eslint 5
…t to JSDOM for specific tests)
… flag, and adding __fixtures__ to the lint exception list made tests fail)
…into eslint-update
🦋 Changeset detectedLatest commit: f3d251a The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
@@ -59,7 +59,7 @@ interface WebSocketMessage { | |||
} | |||
|
|||
connection.onmessage = (m: MessageEvent) => { | |||
const message = JSON.parse(m.data) as WebSocketMessage; |
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.
Might be worth having a runtime safety check here. The simplest upgrade is to make it throw when m.data
isn't a string - since JSON.parse
will already throw in that situation.
I'm not sure what the consequences are of m.data
being non-string - those consequences should drive if this should throw or warn (or do nothing, or something else).
I say all this on the assumption that the MessageEvent
interface is accurate - if data
can be non-string, our code should account for that, probably (or we should know why we don't need to account for it).
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, I tried to improve my understanding of .data and why it's typed as any/if we need to consider it being anything but string.
I opted to follow your suggestion to throw if not string, as JSON.parse will have done anyways. hopefully should never be the case.
@@ -113,6 +113,7 @@ export async function startApp( | |||
// starts above, but don't want to wait until the | |||
// process itself finishes. | |||
return { | |||
// eslint-disable-next-line @typescript-eslint/no-misused-promises |
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.
Why is this promise "misused" and not "floating"? Asking because I think we can silence the "floating" error without eslint comments, by just prepending void
.
@@ -82,6 +82,7 @@ export async function getPackageDependencies(target: string): Promise<{ | |||
|
|||
// Package dependencies can be either local to the package or in the root package (hoisted) | |||
const packageDeps = Object.assign( | |||
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument |
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.
Is this because Object.create(null)
is typed as any
?
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.
Yep so: Unsafe argument of type `any` assigned to a parameter of type `object`.
Object.create() returns any
what would be the cleanest way to deal with this. Using type assertion, typescript suggests:
Don't use Object
as a type. The Object
type actually means "any non-nullish value", so it is marginally better than unknown
.
- If you want a type meaning "any object", you probably want
Record<string, unknown>
instead. - If you want a type meaning "any value", you probably want
unknown
instead.
so Object.create(null) as Record<string, unknown>,
works but doesn't look very clean
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.
This is unfortunately a Typescript "needs proposal" that is currently open: microsoft/TypeScript#1108
People had luck with typing is as no, it doesn't workvoid
or null
, it seems.
so Object.create(null) as Record<string, unknown>, works but doesn't look very clean
What about:
const nullObject: Record<string, unknown> = Object.create(null)
const whatever == Object.assign(nullObject, ...)
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.
Or, more correctly:
const nullObject: Record<keyof Object, never> = Object.create(null)
@@ -5,6 +5,7 @@ export default function memoize<R, T extends (...args: any[]) => R>(f: T): T { | |||
|
|||
const g = (...args: any[]) => { | |||
if (!memory.has(args.join())) { | |||
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument |
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 think you can avoid this by using unknown
instead of any
?
* Node 18 & Jest 29 (#2210) * Add Node 18 to tests * Add Node 18 to supported engines * Pinned rollup-plugin-esbuild to 4.10.1 as newer versions drop support for node 14.17 and below * Updated yarn.lock * Update build.test snapshot * Update modular-scripts snapshot * Change rollup-plugin-eslint to latest compatible version * Added changeset * Update lockfile * Resolve to source-map ^0.7.0 to try and fix mozilla/source-map#432 * Try to resolve v8-to-istanbul to fix mozilla/source-map#432 * Dropped support for Node 14.17.0 (14.18.0 and greater) to allow upgrate to rollup-plugin-eslint 5 * Update tests to use Node 14.18.0 * Make tests use Node 14.18.0 * update jest to 29 * change Node 16 supported version to 16.10+ as that's what Jest supports * Upgrade to Jest 29 and fix problems caused by the upgrade * Update changelog * Update snapshots (default no longer includes escape characters etc) * Update snapshots not to include espace caracters * More snapshots updates * Change env for port tests to Node * Update snapshots and switch default test env to Node (while setting it to JSDOM for specific tests) * Update more snapshots * Remove broken Rename test (Rename functionality to be removed with Modular 4) * Fix tests (test.test wasn't closing after running with the --watchAll flag, and adding __fixtures__ to the lint exception list made tests fail) * Try to fix tests on Windows * DEBUG windows tests * Fix windows tests * Try more things to fix tests * More attempts to fix windows tests * More desperate attempts to fix windows tests * Clean up windows fix * Cleanup * Update changeset * Set Jest watchAll default to false regardless of if CI or not * Added ESLint ignore comments to svgr.ts for new TS complaints * Documentation * Update docs * Add feature/v4 to branches to run CI Tests on (Needs to be reverted before merging to main) * Small fixes * Refactor out generateJestConfig flag logic to reduce duplication * Update to ESLint 8 & Supported TypeScript to >4.5.3 (#2216) * Update ESLint to ^8.0.0, minimum Typescript to 4.5.3 & update dependencies * Update changeset * Fix warnings generated by updated ESLint * Remove commands and update documentation (#2220) * Remove commands and update documentation * Update Readme & Changeset * Merge main (#2225) Merge changes to modular 3.x from main since feature/v4 was split out * Use Config file instead of Environment Variables (#2227) * Use cosmiconfig to read modular configuration, while allowing Env variables to override it * Add configuration tests * Change the default CDN to esm.sh * Update documentation to reflect new configuration approach * Use INTERNAL_ env variables to read configuration in JS files that can't call the config function * Refactor config() logic Co-authored-by: Steve King <[email protected]> * Add jsx to test glob pattern (#2234) * Add jsx to test glob pattern * Merge main into Modular 4 (#2237) * Merge main into Modular 34 * Sunset modular-site * Revert "Sunset modular-site" This reverts commit a27388b. * Sunset modular-site (#2238) * Sunset modular-site & update test snapshot * Remove requirement for Apps to be private in modualr check (#2241) * Update lockfile * Docs/support (#2248) * Initial cleanup * Add compatibility page, remove recipes/yarn * Better markdown section depth * update nav order * Document package types (#2246) * Start documenting package types * Describe App and ESM View types * Packages * Views, sources and templates * Link add command page * Link package types in index * Update docs/commands/add.md Co-authored-by: Sam Brown <[email protected]> * Update docs/concepts/package-types.md Co-authored-by: Sam Brown <[email protected]> * Update docs/concepts/package-types.md Co-authored-by: Sam Brown <[email protected]> * Link esm-views section in add page * Update docs/commands/add.md Co-authored-by: Sam Brown <[email protected]> * Link app section for more info * Update docs/concepts/package-types.md Co-authored-by: Sam Brown <[email protected]> * Clarify manifest -> package.json + fix incorrect description * Add reasons why we don't support things * Disambiguate words * Phrase 'default export' concept better * Update docs/concepts/package-types.md Co-authored-by: Sam Brown <[email protected]> * Update docs/concepts/package-types.md Co-authored-by: Sam Brown <[email protected]> * Update docs/commands/add.md Co-authored-by: Sam Brown <[email protected]> * Reword tricky sentence * Update docs/commands/add.md Co-authored-by: Sam Brown <[email protected]> * Update docs/concepts/package-types.md Co-authored-by: Sam Brown <[email protected]> * Split long sentence * Add all package types to configuration docs * Various typos * Split packge types and move template page * Fix links to package types and fix table layout * Slim down table * build/start/entrypoint/template sections * Fix configuration * Remove list of types in documentation Co-authored-by: Sam Brown <[email protected]> * Generate nicer READMEs inside new projects / apps / views / packages (#2253) * Start documenting package types * Describe App and ESM View types * Packages * Views, sources and templates * Link add command page * Link package types in index * Update docs/commands/add.md Co-authored-by: Sam Brown <[email protected]> * Update docs/concepts/package-types.md Co-authored-by: Sam Brown <[email protected]> * Update docs/concepts/package-types.md Co-authored-by: Sam Brown <[email protected]> * Link esm-views section in add page * Update docs/commands/add.md Co-authored-by: Sam Brown <[email protected]> * Link app section for more info * Update docs/concepts/package-types.md Co-authored-by: Sam Brown <[email protected]> * Clarify manifest -> package.json + fix incorrect description * Add reasons why we don't support things * Disambiguate words * Phrase 'default export' concept better * Update docs/concepts/package-types.md Co-authored-by: Sam Brown <[email protected]> * Update docs/concepts/package-types.md Co-authored-by: Sam Brown <[email protected]> * Update docs/commands/add.md Co-authored-by: Sam Brown <[email protected]> * Reword tricky sentence * Update docs/commands/add.md Co-authored-by: Sam Brown <[email protected]> * Update docs/concepts/package-types.md Co-authored-by: Sam Brown <[email protected]> * Split long sentence * Add all package types to configuration docs * Various typos * Split packge types and move template page * Fix links to package types and fix table layout * Slim down table * build/start/entrypoint/template sections * Add root project README * Add per-package READMEs + fix docs * Lint view documentation * Update snapshots * Upddate app snapshots * Update snapshots in index test * Update app.esbuild.test snapshots * update cmra tests * Correct snapshots for cmra * Update snapshots for cli.test.ts * Remove packages README + fix all READMEs * Add README to fixtures + update snapshots * Update various snapshots * Update app.esbuild.test snapshots * Add default workspace README * Update index snapshots * Create odd-bees-speak.md * Update fixture READMEs Co-authored-by: Sam Brown <[email protected]> Co-authored-by: Alberto Brusa <[email protected]> * Sunset philosophy / views, write intro, fix web workers, correct templates (#2254) * Sunset philopsophy / views, move web workers, correct templates * Better front page * Fix command format * Add web workers docs * Enhance modular-scripts Tests (#2240) * Update tests in modular-scripts to use tmp directories instead of changing things within the repo * Consolidate all execa calls to yarn modular under one standardised function * Change calls to modular during tests to not run checks * Refactor tests for brevity, code reuse and raedability * Remove unnecessary build snapshots from esmVies.test * Modify getConfig to get workspace specific configuration (#2258) * Modify getConfig to get workspace specific configuration * Update config docs to reflect per workspace configuration * Document supported CRA features (#2255) * Rename limitations + start CRA page * Add CRA supported features * Fix typo * Fixes in CRA page * Test default template tests work (#2260) * Unify modular test command interface (#2259) * regex -> option, package -> argument * Windows test use --package * Remove space that fails test * Add debug statement * Implement selective testing and merging with user regexes * Update docs * Better wording * Fix typo * Test selective options combinations * modular build builds all packages * Test selective builds * Document behaviour of modular build * Create green-shrimps-build.md * Fix test docs * various docs fixes (#2262) * Small fixes (#2261) * Remove unnecessary error log * Move jest-environment-jsdom to correct package.json * Update typescript set by CMRA * More small fixes (#2263) * Remove unnecessary error log * Move jest-environment-jsdom to correct package.json * Remove jest-environment-jsdom from root package.json * reorder dependencies * v4.0.2 * v1.1.1 * v4.0.0 * Update typescript set by CMRA * Revert accidentally changed versions and bump CMRA version by major due to update TS version * Remove feature/v4 from GitHub Action tests & introduce forgotten CMRA changeset * Update 4.0.x release notes * Slight changes to release notes * Additions to release doc Co-authored-by: Cristiano Belloni <[email protected]> Co-authored-by: Cristiano Belloni <[email protected]> Co-authored-by: Sam Brown <[email protected]>
Jest 29 JSDOM testing env requires TypeScript 4.5 or greater, ESLint 8 adds support for TypeScript 4.5 or greater.
To support Jest 29, we're upgrading ESLint to 8 and finally freeing the TypeScript version constraints (previously locked)
In the updating dependencies process I also re-generated our lock file hence the updated browser target snapshots