Skip to content

Commit 96a1d03

Browse files
frano-mclaude
andcommitted
fix: code review hardening — blockjs precedence, exports types, readme/link.sh cleanups (#933)
addresses review findings on #942: - staticprops: move blockjs after the ...serializeOptions spread and use serializeOptions.blockJS ?? false so a consumer's blockJS: undefined (from a destructure-respread) no longer silently re-enables next-mdx-remote@6's destructive blockJS: true default. explicit blockJS: true from a consumer is still honoured. - package.json exports: convert the five .js-suffix wildcard entries (./lib/*.js, ./nextauth/*.js, ./google/*.js, ./terra/*.js, ./auth/*.js) from bare-string targets to conditional { types, default } form so node16 / nodenext consumers using explicit .js imports resolve the matching .d.ts instead of falling through to filesystem heuristics. - package.json: add top-level "types": "lib/index.d.ts" next to "main" so legacy moduleResolution "node" consumers (and any tool that ignores exports) find the bare-package type entry. - readme: add import { JSX } from "react" to the _app.tsx and _document.tsx snippets (react 19 removed the global JSX namespace) and export default MyApp; to the _app.tsx snippet (consumer copy-paste needs the default export for the next pages router to pick it up). - readme: flag the --webpack workaround as temporary, point at vercel/next.js#82607 for subscription, and ask consumers to re-review on each next major. - scripts/link.sh: rm -rf lib before tsc so renames and barrel deletions don't leave stale artefacts that get packed into the tarball — would otherwise silently mask breaking-change import paths during local migration testing. updated header comment to reflect actually-packed paths (lib + types, not src). - readme: blank lines around three pre-existing fenced code blocks in the developing-alongside-a-consuming-app section, satisfies md031. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent c291b4d commit 96a1d03

4 files changed

Lines changed: 44 additions & 8 deletions

File tree

README.md

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ Until the upstream fix lands ([vercel/next.js#82607](https://github.com/vercel/n
5151
}
5252
```
5353

54-
Webpack is still fully supported in Next 16 — Turbopack was promoted to default, not "webpack removed." The deprecation timeline for the webpack fallback hasn't been published.
54+
Webpack is still fully supported in Next 16 — Turbopack was promoted to default, not "webpack removed." The deprecation timeline for the webpack fallback hasn't been published, but the `--webpack` flag is a temporary workaround and is expected to be removed in a future major. **Subscribe to [vercel/next.js#82607](https://github.com/vercel/next.js/issues/82607) for status**, and review this pin whenever Next.js publishes a webpack removal notice or the upstream Turbopack + Pages Router + MUI bug is resolved.
5555

5656
This is a Next.js / Turbopack bug, not a findable-ui one. Re-enable Turbopack when [vercel/next.js#82607](https://github.com/vercel/next.js/issues/82607) is closed.
5757

@@ -63,6 +63,7 @@ Wrap the app in `AppCacheProvider`:
6363
import { EmotionCache } from "@emotion/react";
6464
import { AppCacheProvider } from "@mui/material-nextjs/v16-pagesRouter";
6565
import type { AppProps } from "next/app";
66+
import { JSX } from "react";
6667

6768
type MyAppProps = AppProps & {
6869
emotionCache?: EmotionCache;
@@ -77,6 +78,8 @@ function MyApp(props: MyAppProps): JSX.Element {
7778
</AppCacheProvider>
7879
);
7980
}
81+
82+
export default MyApp;
8083
```
8184

8285
### `_document.tsx`
@@ -96,6 +99,7 @@ import Document, {
9699
Main,
97100
NextScript,
98101
} from "next/document";
102+
import { JSX } from "react";
99103

100104
class MyDocument extends Document<DocumentHeadTagsProps> {
101105
render(): JSX.Element {
@@ -145,21 +149,25 @@ into a consuming project (e.g. ncpi-dataset-catalog, data-browser):
145149
2. Set `node` version to `22.13.0`.
146150
3. Run `npm install` in both repositories.
147151
4. From the consuming project directory:
152+
148153
```bash
149154
../findable-ui/scripts/link.sh
150155
```
156+
151157
This compiles TypeScript, packs the output, installs it into
152158
`node_modules`, clears the Next.js cache, and starts the dev server.
153159

154160
5. To iterate: Ctrl+C the dev server, make changes in findable-ui, and
155161
hit up-arrow to re-run the script.
156162

157163
6. To restore the published version:
164+
158165
```bash
159166
../findable-ui/scripts/unlink.sh
160167
```
161168

162169
Consuming projects can optionally add thin wrappers in their `package.json` scripts:
170+
163171
```json
164172
{
165173
"scripts": {

package.json

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
},
2222
"homepage": "https://github.com/DataBiosphere/findable-ui/tree/main/#readme",
2323
"main": "lib/index.js",
24+
"types": "lib/index.d.ts",
2425
"sideEffects": false,
2526
"files": [
2627
"lib",
@@ -31,27 +32,42 @@
3132
"types": "./lib/index.d.ts",
3233
"default": "./lib/index.js"
3334
},
34-
"./lib/*.js": "./lib/*.js",
35+
"./lib/*.js": {
36+
"types": "./lib/*.d.ts",
37+
"default": "./lib/*.js"
38+
},
3539
"./lib/*": {
3640
"types": "./lib/*.d.ts",
3741
"default": "./lib/*.js"
3842
},
39-
"./nextauth/*.js": "./lib/nextauth/*.js",
43+
"./nextauth/*.js": {
44+
"types": "./lib/nextauth/*.d.ts",
45+
"default": "./lib/nextauth/*.js"
46+
},
4047
"./nextauth/*": {
4148
"types": "./lib/nextauth/*.d.ts",
4249
"default": "./lib/nextauth/*.js"
4350
},
44-
"./google/*.js": "./lib/google/*.js",
51+
"./google/*.js": {
52+
"types": "./lib/google/*.d.ts",
53+
"default": "./lib/google/*.js"
54+
},
4555
"./google/*": {
4656
"types": "./lib/google/*.d.ts",
4757
"default": "./lib/google/*.js"
4858
},
49-
"./terra/*.js": "./lib/terra/*.js",
59+
"./terra/*.js": {
60+
"types": "./lib/terra/*.d.ts",
61+
"default": "./lib/terra/*.js"
62+
},
5063
"./terra/*": {
5164
"types": "./lib/terra/*.d.ts",
5265
"default": "./lib/terra/*.js"
5366
},
54-
"./auth/*.js": "./lib/auth/*.js",
67+
"./auth/*.js": {
68+
"types": "./lib/auth/*.d.ts",
69+
"default": "./lib/auth/*.js"
70+
},
5571
"./auth/*": {
5672
"types": "./lib/auth/*.d.ts",
5773
"default": "./lib/auth/*.js"

scripts/link.sh

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,10 @@
1212
# 3. The script builds, installs, and restarts the dev server for you
1313
#
1414
# How it works:
15+
# - Cleans the previous build output (lib/) so renames and deletions don't
16+
# leave stale artefacts in the packed tarball
1517
# - Compiles findable-ui's TypeScript source into lib/
16-
# - Packs lib/, src/, and types/ into a tarball (see "files" in package.json)
18+
# - Packs lib/ and types/ into a tarball (see "files" in package.json)
1719
# - Installs the tarball into the consumer's node_modules WITHOUT modifying
1820
# the consumer's package.json (--no-save), so "npm install" can restore
1921
# the registry version at any time (see unlink.sh)
@@ -31,6 +33,12 @@ CONSUMER_DIR="$PWD"
3133
echo "Building findable-ui from $FINDABLE_DIR..."
3234
cd "$FINDABLE_DIR"
3335

36+
# Wipe the previous build output. tsc doesn't prune deleted/renamed files, so
37+
# stale .js/.d.ts artefacts (e.g. removed barrels or pre-rename filenames)
38+
# would otherwise persist in lib/ and get packed into the tarball — silently
39+
# masking breaking-change import paths during local migration testing.
40+
rm -rf lib
41+
3442
# Compile TypeScript to lib/. Called directly to avoid triggering npm lifecycle
3543
# scripts (e.g. "prepare" would run husky install, which is unnecessary here).
3644
./node_modules/.bin/tsc

src/utils/mdx/staticGeneration/staticProps.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,12 @@ export async function buildStaticProps<F extends object, P extends object>(
3636
// Serialize the MDX content.
3737
const outline: OutlineItem[] = [];
3838
const mdxSource = await serialize(content, {
39-
blockJS: false,
4039
...serializeOptions,
40+
// After the spread so a consumer's `blockJS: undefined` (from a
41+
// destructure-respread) doesn't silently re-enable next-mdx-remote@6's
42+
// destructive `blockJS: true` default. Consumers can still opt in
43+
// explicitly by passing `blockJS: true`.
44+
blockJS: serializeOptions.blockJS ?? false,
4145
mdxOptions: {
4246
development: false,
4347
rehypePlugins: [rehypeSlug],

0 commit comments

Comments
 (0)