Skip to content

chore: Lint browser implementations #2848

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 16 commits into from
Jan 22, 2025
Merged

chore: Lint browser implementations #2848

merged 16 commits into from
Jan 22, 2025

Conversation

teogeb
Copy link
Contributor

@teogeb teogeb commented Oct 25, 2024 •
edited
Loading

Summary

Do not not exclude browser implementations from eslint and check tasks. Removed Browser* files from excludes list of tsconfig.jest.json files.

Other changes

Converted the BrowserPersistence file to a ES module. Before this change we got this error:

src/utils/persistence/BrowserPersistence.ts:1:38 - error TS1479: The current file is a CommonJS module whose imports will produce 'require' calls; however, the referenced file is an ECMAScript module and cannot be imported with 'require'. Consider writing a dynamic 'import("idb")' call instead.
  To convert this file to an ECMAScript module, change its file extension to '.mts', or add the field `"type": "module"` to ...package.json'.
import { openDB, IDBPDatabase } from 'idb'

Alternatively could have tweaked the import? It was not clear what the root cause was, as the idb package seems to support CommonJS: https://github.com/jakearchibald/idb/blob/eb2fc14bb3588d09aaa5e86a83bf3519b06e10b3/package.json#L13

Added DOM types for BrowserWebrtcConnection.

Test

There is no regression for the browser persistence functionality at least with recent Chrome and Firefox versions. We can see from Chrome/Firefox dev tools that this inserts the key to Indexed DB.

<!DOCTYPE html>
<html>
<body>
    <script src="dist/streamr-sdk.web.js"></script>
    <script>
        const client = new StreamrClient()
        client.addEncryptionKey(StreamrClient.EncryptionKey.generate(), '0x1234567890123456789012345678901234567890')
    </script>
</body>
</html>

@github-actions github-actions bot added dht Related to DHT package sdk labels Oct 25, 2024
@teogeb teogeb requested a review from harbu October 25, 2024 09:19
@teogeb teogeb requested a review from harbu October 25, 2024 13:48
@teogeb teogeb mentioned this pull request Oct 25, 2024
Merged
@teogeb teogeb requested review from juslesan and mondoreale October 28, 2024 08:59
Base automatically changed from update-tsconfig-includes to main October 28, 2024 20:25
teogeb added a commit that referenced this pull request Nov 6, 2024
@teogeb
chore: Eslint v9 (#2849)
10e1e41 
## Changes

Converted the configuration file to flat config and unified to config
file style. Otherwise same rules are earlier, except:
- some new rules are now inherited from the updated
`eslint-config-streamr-ts`
  - removed locally defined rules if exactly same as inherited
- converted `cli-tools` logging statements to use `console.info` instead
of `console.log` and removed the `no-console` rule from that package

Fixed lint errors and removed obsolete `eslint-disable` directives. Also
fixed a type in `Operator.test.ts`.

Added `eslint-import-resolver-typescript` dependency. It is needed by
`eslint-plugin-import` (see
https://github.com/import-js/eslint-plugin-import?tab=readme-ov-file#typescript).

## Other PRs

Temporarily disabled linting of browser classes in commit 876423f. That
is needed until we merge
#2848 to `main`. We should
revert the commit either in this PR or in that PR (whichever is landed
to `main` later).
@github-actions github-actions bot added the Stale label Jan 6, 2025
@github-actions github-actions bot closed this Jan 14, 2025
@teogeb teogeb reopened this Jan 14, 2025
@github-actions github-actions bot removed the Stale label Jan 15, 2025
@teogeb teogeb merged commit 5de84ba into main Jan 22, 2025
24 checks passed
@teogeb teogeb deleted the lint-browser-implementations branch January 22, 2025 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@juslesan juslesan juslesan approved these changes

@harbu harbu Awaiting requested review from harbu

@mondoreale mondoreale Awaiting requested review from mondoreale

Assignees
No one assigned
Labels
dht Related to DHT package sdk
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@teogeb @harbu @juslesan