yarn react build|dev #195
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,4 @@ | ||
|
There was a problem hiding this comment. Removing TODOs feels great! |
||
| module.exports = { | ||
| reactStrictMode: true, | ||
| pageExtensions: ['page.tsx'], | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,18 +2,27 @@ | |
| "private": true, | ||
| "name": "@aws-amplify/ui-react", | ||
| "version": "0.0.1", | ||
| "type": "module", | ||
| "main": "dist/index.cjs", | ||
| "module": "dist/index.js", | ||
| "exports": { | ||
| ".": "./dist/index.js", | ||
| "./styles.css": "./dist/index.css" | ||
|
There was a problem hiding this comment.
Neat, right? |
||
| }, | ||
| "types": "dist/index.d.ts", | ||
| "files": [ | ||
| "dist" | ||
| ], | ||
| "scripts": { | ||
| "prebuild": "rimraf dist", | ||
| "build": "tsup --minify", | ||
| "predev": "rimraf dist", | ||
| "dev": "tsup --watch", | ||
| "update:icons": "node scripts/updateIcons.js", | ||
| "build:icons": "yarn run clean:icons && node scripts/generateIcons.js", | ||
| "clean": "rimraf dist", | ||
| "clean:icons": "rimraf ./src/primitives/Icon/icons", | ||
| "postinstall": "patch-package", | ||
| "primitives:new": "hygen primitives new", | ||
| "test": "yarn test:unit", | ||
| "test:watch": "yarn test:unit:watch", | ||
|
|
@@ -26,8 +35,9 @@ | |
| "qrcode": "^1.4.4" | ||
| }, | ||
| "peerDependencies": { | ||
| "aws-amplify": "3.x.x || 4.x.x", | ||
| "react": ">= 16.8.0", | ||
| "react-dom": ">= 16.8.0" | ||
|
Comment on lines
+40
to
+42
There was a problem hiding this comment. This way we don't bundle dependencies. |
||
| }, | ||
| "workspaces": { | ||
| "nohoist": [ | ||
|
|
@@ -36,6 +46,8 @@ | |
| ] | ||
| }, | ||
| "devDependencies": { | ||
| "@aws-amplify/ui-core": "^0.0.1", | ||
| "@aws-amplify/ui-theme-base": "^0.0.1", | ||
|
There was a problem hiding this comment. Build tools like This is helpful for using packages that we don't want to publish. There was a problem hiding this comment. Broader question: do we want to publish ui-theme-base? I noticed we publish @aws-amplify/ui which is kind of similar to what ui-theme-base is now. I don't have a strong opinion, just wanted to bring it up. Should we rename ui-theme-base to just ui? There was a problem hiding this comment. No, any cross-platform/framework dependencies (like the state machine, design tokens, or even specs (we ran into an issue with that!)) ideally lives in But you're right to ask about
The only thing that I found weird is But I have no idea why, or see the need for it. The more I type it out, the more it makes sense for Let's queue this up for another cleanup task. |
||
| "@testing-library/jest-dom": "^5.14.1", | ||
| "@testing-library/react": "^12.0.0", | ||
| "@testing-library/react-hooks": "^7.0.1", | ||
|
|
@@ -47,6 +59,7 @@ | |
| "jest": "^27.0.4", | ||
| "rimraf": "^3.0.2", | ||
| "ts-jest": "^27.0.3", | ||
| "tsup": "^4.14.0", | ||
| "typescript": "^4.3.2" | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| diff --git a/node_modules/tsup/dist/index.js b/node_modules/tsup/dist/index.js | ||
| index 1fd3ea9..8f84cf0 100644 | ||
| --- a/node_modules/tsup/dist/index.js | ||
| +++ b/node_modules/tsup/dist/index.js | ||
| @@ -13633,6 +13633,7 @@ async function runEsbuild(options, { format, css }) { | ||
| jsxFactory: options.jsxFactory, | ||
| jsxFragment: options.jsxFragment, | ||
| sourcemap: options.sourcemap, | ||
| + inject: ['./src/react-shim.js'], | ||
|
There was a problem hiding this comment. |
||
| target: options.target === "es5" ? "es2016" : options.target, | ||
| plugins: [ | ||
| externalPlugin({ | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,5 @@ | ||
| import './styles.css'; | ||
|
There was a problem hiding this comment. This is really nice – building (via PostCSS) styles as part of a single package & process. There was a problem hiding this comment. Dumb question: if we import the styles here, developers using the React components would no longer need to import the styles like There was a problem hiding this comment. Somehow the SCSS stuff just worked?? I need to check the docs to be sure... There was a problem hiding this comment. Good question @dbanksdesign! No, What This is consistent with how, say, an application is bundled with webpack: even though your app has JS, CSS, SVGs, PNGs, whatever, the bundler identifies all dependencies and outputs them based on your configuration. |
||
|
|
||
| export * from './components'; | ||
| export * from './hooks'; | ||
| export * from './primitives'; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| /** | ||
| * This shim exists for `esbuild` to `inject` into every .tsx file. | ||
| * Otherwise, `import * as React from 'react'` would need to be added manually | ||
| * | ||
| * See: https://esbuild.github.io/content-types/#auto-import-for-jsx | ||
| */ | ||
|
|
||
| import * as React from 'react'; | ||
| export { React }; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| @import './components/Authenticator/styles.css'; | ||
| @import './primitives/styles.css'; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,14 +1,16 @@ | ||
| { | ||
| "compilerOptions": { | ||
| "moduleResolution": "node", | ||
| "baseUrl": ".", | ||
|
Comment on lines
+3
to
+4
There was a problem hiding this comment. I thought this was the default, but |
||
| "declaration": true, | ||
| "esModuleInterop": true, | ||
| "jsx": "react-jsx", | ||
| "rootDir": "./src", | ||
| "outDir": "./dist", | ||
| "skipLibCheck": true, | ||
| "sourceMap": true, | ||
| "module": "esnext", | ||
|
There was a problem hiding this comment. So, TIL : https://stackoverflow.com/questions/41993811/understanding-target-and-module-in-tsconfig
|
||
| "target": "es2015" | ||
|
There was a problem hiding this comment.
Targeting evergreen browsers, this could even be But the less TS has to transpile, the faster our builds will be. |
||
| }, | ||
| "exclude": ["node_modules"], | ||
| "include": ["src/**/*.ts", "src/**/*.tsx"] | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| /** | ||
| * @type {import("tsup").Options} | ||
| */ | ||
| module.exports = { | ||
|
There was a problem hiding this comment. I did it this way for type safety vs. |
||
| dts: true, | ||
| entryPoints: ['src/index.tsx'], | ||
| format: ['cjs', 'esm'], | ||
| sourcemap: 'both', | ||
| splitting: false, | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,8 +22,5 @@ | |
| "rimraf": "^3.0.2", | ||
| "sass": "^1.35.2", | ||
| "style-dictionary": "^3.0.1" | ||
|
Comment on lines
-27
to
-29
There was a problem hiding this comment. The bundler had issues with WOFF files inside of this dependency. Admittedly, this is weird to me as part of a library. Because fonts require asset files (e.g. Normally, I see styles only specify
Because fonts can be very expensive, I think we should revisit #165 and try to make bundle-size as much of an opt-in as possible, like https://tailwindui.com/documentation#optional-add-the-inter-font-family. If a customer doesn't want it, If a customer wants to load it in a blocking way, they can use If a customer wants to load it in a non-blocking way, they can do what Next.js does: |
||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ahhh so nice