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

feat(synapse-constants): Refactor with rollup for package build and export [SLT-160] #3175

Merged
merged 14 commits into from
Oct 1, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions packages/synapse-constants/.eslintrc.cjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
module.exports = {
extends: '../../.eslintrc.js',
overrides: [
{
files: ['**/*.ts'],
rules: {
'guard-for-in': 'off',
'prefer-arrow/prefer-arrow-functions': 'off',
},
},
],
}
6 changes: 0 additions & 6 deletions packages/synapse-constants/index.ts

This file was deleted.

42 changes: 20 additions & 22 deletions packages/synapse-constants/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@
"name": "synapse-constants",
"version": "1.3.24",
"description": "This is an npm package that maintains all synapse constants",
"main": "dist/index.js",
"module": "dist/index.js",
"type": "module",
"main": "dist/bundle.cjs.js",
"module": "dist/bundle.esm.js",
"types": "dist/index.d.ts",
"typings": "dist/index.d.ts",
"files": [
"dist"
],
Expand All @@ -16,34 +16,32 @@
"build:slither": " ",
"lint": " ",
"lint:fix": "npm run lint -- --fix",
"lint:check": "eslint . --max-warnings=0",
"build": "node scripts/generateMaps.js && node scripts/findMissing.js",
"prepublish": "tsc",
"compile": "tsc && copyfiles -u 1 \"constants/assets/**/*.*\" dist/constants && webpack",
"maps:generate": "yarn build && yarn compile"
"lint:check": "eslint . --max-warnings=0 --config .eslintrc.cjs",
"prepare": "rollup -c --bundleConfigAsCjs",
"build": "rollup -c --bundleConfigAsCjs",
"prepublish": "yarn build",
"maps:generate": "node ./src/scripts/generateMaps.cjs && node ./src/scripts/findMissing.cjs && yarn build"
},
"author": "",
"license": "ISC",
"dependencies": {
"@codecov/webpack-plugin": "^0.0.1-beta.10",
"copyfiles": "^2.4.1",
"ethers": "5.7.2",
"@ethersproject/bignumber": "5.7.0",
"@ethersproject/address": "5.7.0",
"viem": "^2.21.6",
"lodash": "^4.17.21"
},
"devDependencies": {
"babel-loader": "^9.1.3",
"@codecov/rollup-plugin": "^1.2.0",
"@rollup/plugin-commonjs": "^28.0.0",
"@rollup/plugin-json": "^6.1.0",
"@rollup/plugin-node-resolve": "^15.3.0",
"@rollup/plugin-terser": "^0.4.4",
"@rollup/plugin-url": "^8.0.2",
"babel-plugin-transform-export-extensions": "^6.22.0",
"file-loader": "^6.2.0",
"image-minimizer-webpack-plugin": "^3.8.3",
"imagemin": "^8.0.1",
"imagemin-jpegtran": "^7.0.0",
"imagemin-optipng": "^8.0.0",
"imagemin-svgo": "^10.0.1",
"rollup": "^4.22.4",
"rollup-plugin-typescript2": "^0.36.0",
"svg-inline-loader": "^0.8.2",
"ts-loader": "^9.5.1",
"typescript": "^5.3.3",
"url-loader": "^4.1.1",
"webpack": "^5.89.0",
"webpack-cli": "^5.1.4"
"typescript": "^5.3.3"
}
}
50 changes: 50 additions & 0 deletions packages/synapse-constants/rollup.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import typescript from 'rollup-plugin-typescript2'
import { nodeResolve } from '@rollup/plugin-node-resolve'
import commonjs from '@rollup/plugin-commonjs'
import json from '@rollup/plugin-json'
import terser from '@rollup/plugin-terser'
import url from '@rollup/plugin-url'
import { codecovRollupPlugin } from '@codecov/rollup-plugin'

export default {
input: 'src/index.ts', // Entry point for your constants/utilities
output: [
{
file: 'dist/bundle.cjs.js',
format: 'cjs', // CommonJS output
sourcemap: true,
},
{
file: 'dist/bundle.esm.js',
format: 'esm', // ES Module output
sourcemap: true,
},
],
plugins: [
nodeResolve({
preferBuiltins: true,
}),
commonjs(),
json(),
typescript({
tsconfig: './tsconfig.json',
declaration: true,
declarationDir: 'dist',
}),
terser(),
url({
include: ['**/*.svg', '**/*.png', '**/*.jpg'],
limit: 0,
emitFiles: false,
}),
codecovRollupPlugin({
enableBundleAnalysis: process.env.CODECOV_TOKEN !== undefined,
bundleName: 'synapse-constants',
uploadToken: process.env.CODECOV_TOKEN,
uploadOverrides: {
sha: process.env.GH_COMMIT_SHA,
},
}),
],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM: Plugin configuration is comprehensive and well-structured.

The plugin setup covers all necessary aspects of the build process:

  1. Module resolution and CommonJS conversion.
  2. JSON and TypeScript support.
  3. Minification with terser.
  4. Handling of image files.
  5. Code coverage reporting with Codecov.

The TypeScript plugin is correctly configured with the appropriate tsconfig and declaration settings, which is crucial for maintaining type information.

Consider extracting the Codecov configuration into a separate object for improved readability:

const codecovConfig = {
  enableBundleAnalysis: process.env.CODECOV_TOKEN !== undefined,
  bundleName: 'synapse-constants',
  uploadToken: process.env.CODECOV_TOKEN,
  uploadOverrides: {
    sha: process.env.GH_COMMIT_SHA,
  },
};

// In the plugins array:
codecovRollupPlugin(codecovConfig),

This change would make the configuration more modular and easier to maintain.

external: ['lodash', 'ethers'],
abtestingalpha marked this conversation as resolved.
Show resolved Hide resolved
}
abtestingalpha marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export const findChainIdsWithPausedToken = (routeSymbol: string) => {
PAUSED_TOKENS_BY_CHAIN,
(result, tokens, chainId) => {
if (_.includes(tokens, routeSymbol)) {
result.push(chainId)
result.push(chainId as never)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reconsider the use of as never type assertion

The change from result.push(chainId) to result.push(chainId as never) introduces a potentially problematic type assertion. The never type in TypeScript represents values that never occur, which doesn't align with the expected string type of chainId.

This change might be attempting to bypass a TypeScript compilation error, but it's not addressing the root cause of any potential type mismatch. Instead, consider the following alternatives:

  1. Explicitly type the result array:

    const result: string[] = [];
  2. Use a type guard to ensure chainId is a string:

    if (typeof chainId === 'string') {
      result.push(chainId);
    }
  3. If there's a specific reason for using as never, please add a comment explaining the rationale to improve code maintainability.

}
return result
},
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import synapseLogo from '../assets/icons/syn.svg'
import { Token } from '../types'

import * as CHAINS from '../chains/master'
import {
BUSD,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import sushiLogo from '../assets/icons/sushi.svg'
import { Token } from '../types'

import * as CHAINS from '../chains/master'
import { MINICHEF_ADDRESSES } from '../minichef'

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { BigNumber } from '@ethersproject/bignumber'
import * as CHAINS from '../chains/master'
import { getAddress } from '@ethersproject/address'

import * as CHAINS from '../chains/master'

export type Chain = {
id: number
chainSymbol: string
Expand Down
6 changes: 6 additions & 0 deletions packages/synapse-constants/src/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
export * from './constants/tokens/index'
export * as CHAINS from './constants/chains/index'
export * from './constants/types/index'
// export * from './constants/assets/chains/index'
// export * from './constants/assets/explorer/index'
// export * from './constants/assets/icons/index'
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
const { ethers } = require('ethers')

const { prettyPrintTS } = require('./utils/prettyPrintTs')
const { fetchRfqData } = require('./utils/fetchRfqData')
const { prettyPrintTS } = require('./utils/prettyPrintTs.cjs')
const { fetchRfqData } = require('./utils/fetchRfqData.cjs')
abtestingalpha marked this conversation as resolved.
Show resolved Hide resolved
// Provider URLs
const providers = require('./data/providers.json')
// List of ignored bridge symbols
Expand Down
26 changes: 8 additions & 18 deletions packages/synapse-constants/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,24 +1,14 @@
{
"compilerOptions": {
"outDir": "./dist",
"allowJs": true,
"rootDir": "./",
"module": "ESNext",
"target": "ES2019",
"module": "CommonJS",
"moduleResolution": "node",
"declaration": true,
"outDir": "dist",
"strict": true,
"esModuleInterop": true,
"declaration": false,
"jsx": "react",
"skipLibCheck": true
"resolveJsonModule": true
},
"include": [
"./constants/**/*",
"./custom.d.ts",
"./index.ts",
"./index.d.ts"
],
"exclude": [
"node_modules",
"**/*.spec.ts"
]
}
"include": ["src/**/*"],
"exclude": ["node_modules", "dist"]
}
86 changes: 0 additions & 86 deletions packages/synapse-constants/webpack.config.js

This file was deleted.

Loading
Loading