Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

perf: reduce size of published ganache bundle #798

Closed
wants to merge 9 commits into from
Closed

perf: reduce size of published ganache bundle #798

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
8b36b6b
Make core a dep of cli
honestbonsai Feb 16, 2021
ea6613a
Make external ganache core work in cli
honestbonsai Feb 18, 2021
1ed4f0a
Export the whole name space not just default
honestbonsai Feb 18, 2021
ce66c28
Add comment
honestbonsai Feb 22, 2021
b36bb00
Update src/packages/ganache/index.ts
honestbonsai Feb 23, 2021
477d62d
Update src/packages/cli/package.json
honestbonsai Feb 23, 2021
1ac8e79
Use path.join's relative path instead of resolve's
honestbonsai Feb 26, 2021
3bba17e
Export named server and provider from dist/node
honestbonsai Mar 2, 2021
b0fe10e
Merge branch 'develop' into feature/cli-bundle-slimming
davidmurdoch May 18, 2021
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
16 changes: 15 additions & 1 deletion src/packages/ganache/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,18 @@
import Ganache from "@ganache/cli";
/*
* This file is the entry point for the resultant bundle dist/node/ganache.min.js
* dist/cli/ganache.min.js will then point to dist/node/ganache.min.js
* whenever it references @ganache/core.
* This is so we avoid an extra set of native node modules in dist/cli, just use what's in dist/node.
*/
import Ganache from "@ganache/core";

export { serverDefaults } from "@ganache/core";

export const server = Ganache.server;

// Need to add this type annotation of Ganache.provider
// to prevent a TS error about inferred types and not being portable
export const provider: typeof Ganache.provider = Ganache.provider;

export default {
/**
Expand Down
1 change: 1 addition & 0 deletions src/packages/ganache/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
"truffle"
],
"devDependencies": {
"@ganache/core": "^0.1.0",
"@ganache/cli": "^0.1.0",
"@ganache/flavors": "^0.1.0",
"@types/node": "14.14.6",
Expand Down
4 changes: 4 additions & 0 deletions src/packages/ganache/tsconfig.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@
"name": "@ganache/flavors",
"path": "../flavors"
},
{
"name": "@ganache/core",
"path": "../core"
},
{
"name": "@ganache/filecoin",
"path": "../../chains/filecoin/filecoin"
Expand Down
10 changes: 9 additions & 1 deletion src/packages/ganache/webpack/webpack.cli.config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,15 @@ const config: webpack.Configuration = merge({}, base, {
output: {
path: path.resolve(__dirname, "../", "dist", "cli")
},
externals: ["bigint-buffer", "leveldown", "secp256k1", "keccak"],
externals: [
"bigint-buffer",
"leveldown",
"secp256k1",
"keccak",
{
"@ganache/core": path.join("../", "node", "ganache.min.js")
}
],
plugins: [
new webpack.BannerPlugin({ banner: "#!/usr/bin/env node", raw: true })
],
Expand Down
1 change: 0 additions & 1 deletion src/packages/ganache/webpack/webpack.common.config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ const base: webpack.Configuration = {
output: {
filename: "ganache.min.js",
library: "Ganache",
libraryExport: "default",
Copy link
Member

Choose a reason for hiding this comment

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

Any thoughts on if this change has an effect elsewhere?

Copy link
Author

Choose a reason for hiding this comment

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

The only change I noticed is that it manages to export the named exports in addition to the default export. I think if this is set to default, it only exports the default export.

Copy link
Author

@honestbonsai honestbonsai Feb 23, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

If we want to be really safe, I can isolate this change to just webpack.node.config

Copy link
Member

Choose a reason for hiding this comment

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

Ah, ok.

Can you run some smoke tests locally to ensure we can still import and require both the node and cli bundles?

basically:

// index.js
const ganache = require("{path-to-file}/ganache.min.js");
console.log(ganache) // should contain `provider` and `server` (`default` show up here, too)
// index.ts, with no/default tsconfig.json (i.e., `esModuleInterop` is set to `false`)
import ganache from "{path-to-file}/ganache.min.js";
console.log(ganache) // should contain `provider` and `server`

Copy link
Member

@davidmurdoch davidmurdoch Feb 24, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

important Note: I have NOT checked if this is how it behaved prior to your changes, but it's how I'd like it to behave, if possible. :-)

Copy link
Member

Choose a reason for hiding this comment

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

I don't think it really makes sense to have it exposed to the public interface at the level it is now. I'd really prefer if it wasn't exposed to users at all, but that'd likely be a lot more work.

Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we can easily move serverDefaults from packages/core over to packages/cli; that way we won't need to expose it at all!

Copy link
Author

Choose a reason for hiding this comment

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

I wonder if we can easily move serverDefaults from packages/core over to packages/cli; that way we won't need to expose it at all!

I don't think this is possible as core uses it in src/packages/core/src/options/index.ts

Copy link
Author

Choose a reason for hiding this comment

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

important Note: I have NOT checked if this is how it behaved prior to your changes, but it's how I'd like it to behave, if possible. :-)

fyi this is how it is currently in develop

node test.js 
{ server: [Function: server], provider: [Function: provider] }

Copy link
Author

@honestbonsai honestbonsai Mar 2, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

After exported a named server and provider and patching the inferred type issue

node test.js 
{ serverDefaults: [Getter],
  server: [Function: server],
  provider: [Function: provider],
  default:
   { server: [Function: server], provider: [Function: provider] } }

libraryTarget: "umd"
},
stats: {
Expand Down