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

Use preconstruct for building releases #24

Merged
merged 1 commit into from
Dec 1, 2021

Conversation

etr2460
Copy link
Contributor

@etr2460 etr2460 commented Nov 29, 2021

This PR is an alternate to #23 where we use preconstruct for building the release. This entails:

  • Using babel for transpiling the code (.babelrc file added)
  • Bundling all the code into a single, non minified file (both cjs and esm supported)
  • Possibly makes monorepo work easier in the future

I'll keep these both open, so that folks can see the two options and pick whichever one makes the most sense for the project

New ./dist output:
image

to: @Dhaiwat10 @crondinini @with-heart

@etr2460 etr2460 force-pushed the erik-ritter--preconstruct branch from 429b4b4 to a9265bd Compare November 29, 2021 02:41
@Dhaiwat10
Copy link
Member

Interesting, thanks for taking out the time to do all the research and give us options to choose from! I'm looking at a .dev.js file in the build output. I also see that it has a separate .prod.js file. What's going on there?

@etr2460
Copy link
Contributor Author

etr2460 commented Nov 29, 2021

I'm looking at a .dev.js file in the build output. I also see that it has a separate .prod.js file. What's going on there?

@Dhaiwat10 that's a very good question. Right now, the contents of web3-ui.cs.js is:

'use strict';

if (process.env.NODE_ENV === "production") {
  module.exports = require("./web3-ui.cjs.prod.js");
} else {
  module.exports = require("./web3-ui.cjs.dev.js");
}

however, the prod and dev versions are both identical. according to the preconstruct docs:

Note: This file actually just reexports either a production or a development bundle (respectively dist/my-package.cjs.prod.js or dist/my-package.cjs.dev.js in this example) based on the process.env.NODE_ENV value. This allows you to use process.env.NODE_ENV checks in your code so 2 distinct bundles are created but at runtime only one of them gets loaded.

I'm not really sure why all this exists though. Maybe @with-heart could illuminate

package.json Outdated
Comment on lines 21 to 22
"build": "yarn clean; preconstruct build; cp LICENSE package.json README.md ./dist",
"clean": "rimraf dist",
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"build": "yarn clean; preconstruct build; cp LICENSE package.json README.md ./dist",
"clean": "rimraf dist",
"build": "preconstruct build",

preconstruct cleans as part of its internal process. We also don't need to copy anything into dist, we can just include LICENSE and README.md later as part of the published module using the files key (package.json is always included).

@with-heart
Copy link
Member

No idea about dev vs. prod files. Not a feature I've needed to use before so not sure what it does.

@with-heart
Copy link
Member

When we switch this over to support the monorepo, we'll want to add a script:

"postinstall": "preconstruct dev"

This creates files in dist that export directly from src, which means that in local dev we can make a change in one package and use that change in the other package without having to re-build anything.

imo that's one of the killer features of preconstruct. With tsc or other build methods, we'd have to rebuild a package every time we changed it in order to use those changes in one of the other packages. With preconstruct we just make the change and then it's immediately available.

@etr2460 etr2460 force-pushed the erik-ritter--preconstruct branch from a9265bd to 8bdb243 Compare November 30, 2021 04:49
@etr2460
Copy link
Contributor Author

etr2460 commented Nov 30, 2021

@with-heart this is now rebased on top of the monorepo work, and thus is ready for another review. The one issue I had was trying to build packages using a shared babel config from the root of the repo. I got the below error, and couldn't figure out how to resolve it, so i figured i'd just keep around the package level build commands for now. maybe we can fix this in the future, but this should be good to go otherwise.

Error message below:
image

@etr2460 etr2460 force-pushed the erik-ritter--preconstruct branch from 8bdb243 to 5570a48 Compare December 1, 2021 05:29
@etr2460 etr2460 force-pushed the erik-ritter--preconstruct branch from 5570a48 to 506ca11 Compare December 1, 2021 06:11
@etr2460
Copy link
Contributor Author

etr2460 commented Dec 1, 2021

i'm going to merge this so that development is easier (no need to keep running yarn build). we can revisit the build/babel configs when we start automating releases

@etr2460 etr2460 merged commit a564662 into Developer-DAO:master Dec 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants