chore(build): move tsdx to esbuild #957

hardfist · 2021-03-29T01:34:25Z

just try what esbuild cann't be done for author libraries

netlify · 2021-03-29T01:37:03Z

Deploy preview for redux-starter-kit-docs ready!

Built with commit 7c6f46d

https://deploy-preview-957--redux-starter-kit-docs.netlify.app

codesandbox-ci · 2021-03-29T01:37:38Z

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 7c6f46d:

Sandbox	Source
Vanilla	Configuration
Vanilla Typescript	Configuration

markerikson · 2021-03-29T01:44:28Z

Hey, thanks for putting this together.

Unfortunately, we do still currently include a UMD build in our published packages, and I don't think ESBuild supports that at all.

hardfist · 2021-03-29T01:46:48Z

Hey, thanks for putting this together.

Unfortunately, we do still currently include a UMD build in our published packages, and I don't think ESBuild supports that at all.

done!, I just need to convert bundled esm to umd by rollup

markerikson · 2021-03-29T01:53:16Z

So there's a couple other nuances to be aware of with our UMD builds.

We currently output both a dev and prod UMD build.

Not only is the prod UMD build minified, but we need to strip out a couple bits of code from getDefaultMiddleware.ts that are dev-mode only. Currently, we use a rollup-plugin-strip-code to do that.

hardfist · 2021-03-29T02:52:51Z

So there's a couple other nuances to be aware of with our UMD builds.

We currently output both a dev and prod UMD build.

Not only is the prod UMD build minified, but we need to strip out a couple bits of code from getDefaultMiddleware.ts that are dev-mode only. Currently, we use a rollup-plugin-strip-code to do that.

only form umd production ? what about esm&cjs production?

markerikson · 2021-03-29T03:08:35Z

Uh... just UMD, I think? :) It's getDefaultMiddleware.ts, specifically.

That file's gone through a few different changes over time - you can see the changes in the git history.

hardfist · 2021-03-29T03:44:13Z

Uh... just UMD, I think? :) It's getDefaultMiddleware.ts, specifically.

That file's gone through a few different changes over time - you can see the changes in the git history.

I use esbuild plugin to handle this, I think it's ok now, but I do not know how to test

hardfist · 2021-03-29T03:47:35Z

scripts/build.js

+            const source = await fs.readFileSync(args.path, 'utf-8')
+            const defaultPattern = /\/\* PROD_START_REMOVE_UMD[\s\S]*?\/\* PROD_STOP_REMOVE_UMD \*\//g
+            const code = source.replace(defaultPattern, '')
+            // if (sourceMap !== false && sourcemap !== false) {


need to handle sourcemap

markerikson · 2021-03-29T03:47:53Z

SO FYI, I think the "replace chunk of code" behavior could be accomplished using ESBuild's "define" replacement capability.

Specifically, if we set up the definitions such that PROD_START_REMOVE_UMD */ and /* PROD_STOP_REMOVE_UMD were replaced with nothing, that would leave these code blocks commented out.

To test this behavior, we'd want to do some manual searching in the production UMD file to see if any contents from the immutability middleware are in there.

(Side note: it actually looks like the current getDefaultMiddleware.ts is actually a bit buggy in that respect - we really should be cutting out the serializability middleware in prod UMD builds using that same trick, and we're currently leaving it in.)

Also: it looks like the use of tsc is outputting a bunch of individual JS files to dist. Playing with your branch locally, I think we want:

"emitDeclarationOnly": true,

markerikson · 2021-03-29T03:56:22Z

So there's one other point of concern here.

I believe ESBuild only outputs "modern" JS ( evanw/esbuild#297 ). Right now, our CommonJS and ESM builds are back-compiled to cover ES5 and IE11, as far as I know.

I'm all in favor of dropping IE11/ES5-only support in general, but I'm not sure we can pull the trigger on that just yet. I don't know what the right semver rules are for that sort of thing.

hardfist · 2021-03-29T03:56:24Z

i check the code length to ensure works，and check the require result of umd bundle

hardfist · 2021-03-29T03:57:47Z

So there's one other point of concern here.

I believe ESBuild only outputs "modern" JS ( evanw/esbuild#297 ). Right now, our CommonJS and ESM builds are back-compiled to cover ES5 and IE11, as far as I know.

I'm all in favor of dropping IE11/ES5-only support in general, but I'm not sure we can pull the trigger on that just yet. I don't know what the right semver rules are for that sort of thing.

i normally using ts transpilemodule to transform esbuild bundle result to target es5，if you prefer i can change this

markerikson · 2021-03-29T04:07:30Z

Yeah, that was gonna be my next train of thought - transpiling the built output itself in order to enable running okay in IE11, vs transpiling the input files straight to ES5.

So here's two other things that would be useful to sort out.

First, right now TSDX is generating three files for CJS: redux-toolkit.cjs.develeopment.js, redux-toolkit.cjs.production.js, and an index.js file that conditionally imports one of those two:

https://unpkg.com/browse/@reduxjs/toolkit@1.5.1/dist/

Right now this PR is generating the dev and prod files, but the dev file is missing the .development portion of the name, and we don't have the index.js file being generated.

Also: part of the reason why we're wanting to revamp the build system is that we're going to need to support additional entry points. In particular, we're going to be adding support for:

import {thing1} from "@reduxjs/toolkit/query"
import {thing2} from "@reduxjs/toolkit/query/react"

Any chance you could put together an example of how that might work here?

btw, really do appreciate you working with me on fleshing out this idea :) I was skeptical at first, but it does seem like this could work.

hardfist · 2021-03-29T05:02:49Z

add additional entry points seems to need use NodeJS 12.7+ package exports, which typescript does not supports yet, microsoft/TypeScript#33079, and package subpath exports is actually an breaking change

markerikson · 2021-03-29T05:26:58Z

Hmm. We seem to have something working for multiple entry points over in the RTK Query repo, although admittedly we haven't published a real version with that build config - just the auto builds per PR.

hardfist · 2021-03-29T05:28:36Z

add index.js entry points, down-level to es5, remove extra .js files

hardfist · 2021-03-29T05:38:25Z

Hmm. We seem to have something working for multiple entry points over in the RTK Query repo, although admittedly we haven't published a real version with that build config - just the auto builds per PR.

I check the RTK query docs, didn't find any docs about additional entry points, could you give me an example, so I can do the same things here.

hardfist · 2021-03-29T05:38:53Z

scripts/build.js

+    }js`,
+    write: false,
+    target: 'es2015',
+    minify: env === 'production',


todo sourcemap merge

markerikson · 2021-03-29T05:56:41Z

The ultimate plan here is to have three entry points:

"@reduxjs/toolkit": standard RTK APIs ( configureStore, createSlice )
"@reduxjs/toolkit/query": RTK Query UI-agnostic version of createApi plus utils
"@reduxjs/toolkit/query/react": React-specific version of createApi that auto-generates React hooks too

You can see some of that config here:

https://github.com/rtk-incubator/rtk-query/blob/783a2ab7bfa6c966bffa86e17fe55a7b2f250c7e/rollup.config.js

and here's a CI build from rtk-incubator/rtk-query#177 so you can see the rough build output:

rtk-query-pr-177.zip

We'll need something sorta like that in order to release 1.6 the way we want to, but we're open to exactly how that gets done.

hardfist · 2021-03-29T06:21:34Z

The ultimate plan here is to have three entry points:

"@reduxjs/toolkit": standard RTK APIs ( configureStore, createSlice )

"@reduxjs/toolkit/query": RTK Query UI-agnostic version of createApi plus utils

"@reduxjs/toolkit/query/react": React-specific version of createApi that auto-generates React hooks too

You can see some of that config here:

https://github.com/rtk-incubator/rtk-query/blob/783a2ab7bfa6c966bffa86e17fe55a7b2f250c7e/rollup.config.js

and here's a CI build from rtk-incubator/rtk-query#177 so you can see the rough build output:

rtk-query-pr-177.zip

We'll need something sorta like that in order to release 1.6 the way we want to, but we're open to exactly how that gets done.

put an package in another package ,clever idea, I will try this method

hardfist · 2021-03-29T13:25:53Z

@markerikson it seems that query && query/react is in another repo, then How should I put the repo code here, so you want to use monorepo to put rtk/query code into RTK?

markerikson · 2021-03-29T13:40:30Z

We'll be migrating the RTK Query code over into this repo in the near future, but it's not ready yet. For now, could you temporarily just add a couple dummy source files and use those as the source for two new subpackages, just to see if we can get this to work?

Ideally, one subpackage would export a plain JS function, and the other subpackage would export both that first function and a custom React hook, because that is what we'll be doing later.

hardfist · 2021-03-31T02:54:28Z

yes, exports definitely will break code under nodejs, but I'm little confused, is it really necesary to split file to index.js|xxx.development|xxx.production other than just ship an index.js which contains development and production code, since bundler will eventually treeshake the unused code according to process.env.NODE_ENV, and node.js project seems don't care so much about the size

markerikson · 2021-03-31T03:00:41Z

So in theory, bundlers should be picking up the .esm file, with its process.env values still embedded.

CJS is a fallback for Node. The split-file convention is something React has been using for a while, so we have too.

The .modern build is ESM, but with modern ES2017 syntax.

However, per #652 , files with process.env don't work in a browser. So, I added .modern.development and .modern.production files to support loading those in a browser directly (via Unpkg or similar).

markerikson · 2021-03-31T03:11:09Z

@hardfist okay, I was able to install the CSB-built package into a local CRA project, but there's something buggy about the imports/exports in redux-toolkit.esm.js:

Failed to compile.

./node_modules/@reduxjs/toolkit/dist/redux-toolkit.esm.js
Attempted import error: 'Draft' is not exported from 'immer'.

I see two sets of exports in that file. One is around like 75, like this:

import { enableES5 } from "immer";
export * from "redux";
import { default as default2, Draft as Draft2, current as current2, freeze, original, isDraft as isDraft4 } from "immer";
import { createSelector as createSelector2, Selector, OutputParametricSelector, OutputSelector, ParametricSelector } from "reselect";
// src/createDraftSafeSelector.ts
import { current, isDraft } from "immer";
import { createSelector } from "reselect";

Notice that a bunch of those are actually TS types, and this file should not be exporting those.

then, at the bottom, there's:

export { Draft2 as Draft, MiddlewareArray, OutputParametricSelector, OutputSelector, ParametricSelector, Selector, 
ThunkAction, ThunkDispatch, configureStore, createAction, createAsyncThunk, createDraftSafeSelector, 
createEntityAdapter, createImmutableStateInvariantMiddleware, default2 as createNextState, createReducer, 
createSelector2 as createSelector, createSerializableStateInvariantMiddleware, createSlice, current2 as current, 
findNonSerializableValue, freeze, getDefaultMiddleware, getType, isAllOf, isAnyOf, isAsyncThunkAction, 
isDraft4 as isDraft, isFulfilled, isImmutableDefault, isPending, isPlain, isPlainObject, isRejected, 
isRejectedWithValue, nanoid, original, unwrapResult };

which is A) duplicate, and B) still exporting a bunch of TS types that should not be exported.

It's getting late here and I gotta head to bed. Can you figure out what's going on there and fix it? Thank you!

hardfist · 2021-03-31T03:11:34Z

haha,react doesn't support esm yet, so the bundle actually use cjs version of react, the esm split make sense to me now, since to support bundleness scenario

hardfist · 2021-03-31T03:15:24Z

I will check it , I just get up

markerikson · 2021-03-31T03:21:44Z

I DEMAND THAT YOU FIX THIS NOW, BEFORE BREAKFAST!!!

:)

(seriously, again, appreciate all your efforts on this!)

hardfist · 2021-03-31T04:14:28Z

It's kind of tricky here, since esbuild does not support shakes implicit type import|export, so we can change all type export to explicit type export or waiting esbuild to support this feature.
and the duplicate export seems not a problem(even thou it's kind of nasty here), since it exports different things here.

markerikson · 2021-03-31T04:17:40Z

I'm fine with changing whatever type import/export declarations we need to make this work - please go ahead!

hardfist · 2021-03-31T05:33:22Z

woops, it seems that api-extractor doesn't supports type export yet

hardfist · 2021-03-31T05:54:42Z

I currently disable api-extractor and ship all dts files instead, and it passes the redux-typescript cra template test. api-extractor seems needs some hack.

hardfist · 2021-03-31T06:14:16Z

after I upgrade api-extractor to latest version, it works now

hardfist · 2021-03-31T06:20:58Z

Oh, I don't know you have to support typescript version below 3.8 which doesn't support type export|import, can we drop support for this typescript version ?

phryneas · 2021-03-31T06:48:19Z

Given that by now 4.2 is out, that would still fall under "support the last 5 TS versions", so I'd be fine with it.

markerikson · 2021-03-31T14:05:52Z

Yeah, we can drop TS <3.8 for RTK 1.6

I thought of another thing I'd like us to look at as well, although it doesn't strictly have to be in this PR (could be in a follow-up).

Over in reduxjs/redux#3920 , we're updating the Redux core to extract error messages and replace them with error codes in production builds, same as how React does it. That uses a semi-standalone Babel-powered script.

How feasible would it be to apply the same changes to RTK once we're using ESBuild?

Although, having said that... I don't think we even have much in the way of error messages in RTK itself that currently end up in the prod build. There's "invalid reducer" in configureStore, but about the only other error messages I see are in the dev check middleware and those get excluded in prod builds anyway. So, this may be an irrelevant train of thought.

hardfist · 2021-03-31T15:05:24Z

It's easy just like before, all we need to do is to use babel with the previous plugin to transform the bundled result, but I don't like this babel trick actually , I would prefer follow what typescript handle error message, and it's good for search engine

hardfist · 2021-03-31T15:17:22Z

I think we may need some sourcemap test to verify the sourcemap like esbuild , since the transform is kind of complex here, there may be something wrong with production sourcemappings here

markerikson · 2021-03-31T15:49:07Z

Yeah, not sure how you'd test that, but seems like a good idea. Anything we can do to better verify the contents of the build artifacts is a good thing.

# Conflicts: # src/entities/sorted_state_adapter.test.ts

markerikson · 2021-04-01T02:32:11Z

Awright. At this point we have:

All of our original artifacts, approximately the same size
The additional "modern" artifacts
Sourcemaps
Linting + formatting + tests
Examples of generating subpackages
Dropping TS < 3.8
Updated GHA+CSB to Node 14

I've installed the CSB-built package locally into a CRA app and it runs, and I see the latest built .esm file in the DevTools source debugger.

Got at least one thumbs-up on Twitter that the CSB build runs.

I think this PR is at least good to merge in, and we can do any follow-up work in additional PRs if needed.

I'm going to go ahead and merge this.

@hardfist , THANK YOU for your hard work putting this together!

Andarist · 2021-04-01T05:38:28Z

scripts/build.js

+      minify: false,
+      env: '',
+    },
+    // ESM, embedded `process`, ES2017 syntax: modern Webpack dev


How a webpack user can use this bundle?

hardfist force-pushed the move-to-esbuild branch from 6adf5f1 to 838e322 Compare March 29, 2021 01:46

hardfist force-pushed the move-to-esbuild branch from 838e322 to c4daceb Compare March 29, 2021 02:50

hardfist force-pushed the move-to-esbuild branch 2 times, most recently from e671753 to 8baad0f Compare March 29, 2021 03:43

hardfist commented Mar 29, 2021

View reviewed changes

markerikson mentioned this pull request Mar 29, 2021

Future planning: RTK 2.0? #958

Closed

hardfist force-pushed the move-to-esbuild branch from 8baad0f to bc1e816 Compare March 29, 2021 05:27

hardfist commented Mar 29, 2021

View reviewed changes

scripts/build.js Outdated

}js`,

write: false,

target: 'es2015',

minify: env === 'production',

Copy link

Contributor Author

hardfist Mar 29, 2021

Choose a reason for hiding this comment

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

todo sourcemap merge

hardfist force-pushed the move-to-esbuild branch from bc1e816 to ee0fc59 Compare March 29, 2021 10:32

Bump CodeSandbox-CI Node version to 14

50220d1

Fix wrong Terser dep

57c9a32

hardfist mentioned this pull request Mar 31, 2021

esbuild does not tree shake implicit type import evanw/esbuild#1092

Closed

fix: fix api-extractor && type export problem

7dbef9f

hardfist force-pushed the move-to-esbuild branch from a25cc46 to 7dbef9f Compare March 31, 2021 06:13

markerikson linked an issue Apr 1, 2021 that may be closed by this pull request

The cjs bundle is not fully es5 compatible #901

Closed

markerikson mentioned this pull request Apr 1, 2021

The cjs bundle is not fully es5 compatible #901

Closed

markerikson added 2 commits March 31, 2021 21:42

Merge branch 'master' into move-to-esbuild

65d784e

# Conflicts: # src/entities/sorted_state_adapter.test.ts

Drop TS < 3.8 from test matri

7c6f46d

markerikson merged commit cb0535d into reduxjs:master Apr 1, 2021

Andarist reviewed Apr 1, 2021

View reviewed changes

markerikson mentioned this pull request Jul 3, 2021

React-Redux Roadmap: version 8.x, React 18, and TypeScript reduxjs/react-redux#1740

Closed

chore(build): move tsdx to esbuild #957

chore(build): move tsdx to esbuild #957

Conversation

hardfist commented Mar 29, 2021

netlify bot commented Mar 29, 2021 • edited

codesandbox-ci bot commented Mar 29, 2021 • edited

markerikson commented Mar 29, 2021

hardfist commented Mar 29, 2021

markerikson commented Mar 29, 2021

hardfist commented Mar 29, 2021

markerikson commented Mar 29, 2021

hardfist commented Mar 29, 2021

hardfist Mar 29, 2021

Choose a reason for hiding this comment

markerikson commented Mar 29, 2021 • edited

markerikson commented Mar 29, 2021

hardfist commented Mar 29, 2021

hardfist commented Mar 29, 2021

markerikson commented Mar 29, 2021

hardfist commented Mar 29, 2021

markerikson commented Mar 29, 2021

hardfist commented Mar 29, 2021

hardfist commented Mar 29, 2021

hardfist Mar 29, 2021

Choose a reason for hiding this comment

markerikson commented Mar 29, 2021

hardfist commented Mar 29, 2021

hardfist commented Mar 29, 2021

markerikson commented Mar 29, 2021

hardfist commented Mar 31, 2021

markerikson commented Mar 31, 2021 • edited

markerikson commented Mar 31, 2021 • edited

hardfist commented Mar 31, 2021 • edited

hardfist commented Mar 31, 2021

markerikson commented Mar 31, 2021

hardfist commented Mar 31, 2021 • edited

markerikson commented Mar 31, 2021

hardfist commented Mar 31, 2021

hardfist commented Mar 31, 2021

hardfist commented Mar 31, 2021

hardfist commented Mar 31, 2021

phryneas commented Mar 31, 2021

markerikson commented Mar 31, 2021 • edited

hardfist commented Mar 31, 2021 • edited

hardfist commented Mar 31, 2021

markerikson commented Mar 31, 2021 • edited

markerikson commented Apr 1, 2021

Andarist Apr 1, 2021

Choose a reason for hiding this comment

netlify bot commented Mar 29, 2021 •

edited

codesandbox-ci bot commented Mar 29, 2021 •

edited

markerikson commented Mar 29, 2021 •

edited

markerikson commented Mar 31, 2021 •

edited

markerikson commented Mar 31, 2021 •

edited

hardfist commented Mar 31, 2021 •

edited

hardfist commented Mar 31, 2021 •

edited

markerikson commented Mar 31, 2021 •

edited

hardfist commented Mar 31, 2021 •

edited

markerikson commented Mar 31, 2021 •

edited