search

markerikson / react-prod-sourcemaps

A tool to update app sourcemaps with the original code of ReactDOM's production builds
MIT License
57 stars 2 forks source link

Bundling doesn't seem to be right #9

Closed jasikpark closed 1 year ago

jasikpark commented 1 year ago

When trying to integrate the Vite plugin, I get this error:

webclient on  update-react-sourcemaps-tool [!] via  v18.16.0 
❯ pnpm build

> nebula-web@0.0.0 build /Users/calebjasik/Git/defined.net/webclient
> vite build && pnpm post-build

failed to load config from /Users/calebjasik/Git/defined.net/webclient/vite.config.ts
error during build:
file:///Users/calebjasik/Git/defined.net/webclient/vite.config.ts.timestamp-1694618874841-ea67fb622aaf5.mjs:3
import { ViteReactSourcemapsPlugin } from "file:///Users/calebjasik/Git/defined.net/webclient/node_modules/.pnpm/@acemarke+react-prod-sourcemaps@0.0.2_patch_hash=jpyptpvw6t3tnchzyqe4intg7m/node_modules/@acemarke/react-prod-sourcemaps/lib/index.js";
         ^^^^^^^^^^^^^^^^^^^^^^^^^
SyntaxError: Named export 'ViteReactSourcemapsPlugin' not found. The requested module 'file:///Users/calebjasik/Git/defined.net/webclient/node_modules/.pnpm/@acemarke+react-prod-sourcemaps@0.0.2_patch_hash=jpyptpvw6t3tnchzyqe4intg7m/node_modules/@acemarke/react-prod-sourcemaps/lib/index.js' is a CommonJS module, which may not support all module.exports as named exports.
CommonJS modules can always be imported via the default export, for example using:

import pkg from 'file:///Users/calebjasik/Git/defined.net/webclient/node_modules/.pnpm/@acemarke+react-prod-sourcemaps@0.0.2_patch_hash=jpyptpvw6t3tnchzyqe4intg7m/node_modules/@acemarke/react-prod-sourcemaps/lib/index.js';
const { ViteReactSourcemapsPlugin } = pkg;

    at ModuleJob._instantiate (node:internal/modules/esm/module_job:124:21)
    at async ModuleJob.run (node:internal/modules/esm/module_job:190:5)
 ELIFECYCLE  Command failed with exit code 1.

webclient on  update-react-sourcemaps-tool [!] via  v18.16.0 
❯

which matches the errors described in https://arethetypeswrong.github.io/?p=%40acemarke%2Freact-prod-sourcemaps%400.0.5

markerikson commented 1 year ago

Yeah, right now it's just doing a dumb tsc compile, not anything fancier.

What tsconfig module options are you using there?

jasikpark commented 1 year ago 
{
  "compilerOptions": {
    "module": "ESNext",
    "target": "ESNext",
    "moduleResolution": "node",
    "esModuleInterop": true,
    "jsx": "preserve",
    "noEmit": true,
    // Typescript 4.0 allows incremental typechecks: https://devblogs.microsoft.com/typescript/announcing-typescript-4-0-beta/#incremental-with-noemit
    "incremental": true,
    "baseUrl": ".",
    "typeRoots": ["node_modules", "types"], // https://github.com/microsoft/TypeScript/issues/54629#issuecomment-1589593147
    "types": ["vite/client", "vite-plugin-svgr/client"],
    "strict": true,
    "noImplicitOverride": true,
    "skipLibCheck": true,
    "forceConsistentCasingInFileNames": true,
    "noFallthroughCasesInSwitch": true,
    "resolveJsonModule": true,
    "noUncheckedIndexedAccess": true,
    "verbatimModuleSyntax": true,
    "plugins": [{ "name": "typescript-plugin-css-modules" }],
    // If you add any more aliases here, be sure to also add it to .prettierc.cjs
    "paths": {
      "@api/*": ["src/api/*"],
      "@api": ["src/api/"],
      "@assets/*": ["src/assets/*"],
      "@components/*": ["src/components/*"],
      "@global/*": ["src/global/*"],
      "@hooks/*": ["src/hooks/*"],
      "@mock/*": ["mock/*"],
      "@screens/*": ["src/screens/*"],
      "@test-setup/*": ["test-setup/*"],
      "@dn-types/*": ["types/*"],
      "@util/*": ["src/util/*"]
    }
  },
  "include": ["src", "types", "test-setup", "mock", "e2e"],
  "exclude": ["functions/**/*"]
}
jasikpark commented 1 year ago
image

I think this is an indication that something's a bit weird somewhere though?

markerikson commented 1 year ago

Yeah, that's my point. Thus far I haven't put any effort into "bundling" the output or trying to improve the package.exports stuff. It's just straight tsc compilation output.

jasikpark commented 1 year ago

hmm, ok, I may try to muck around and sort it out thenn

jasikpark commented 1 year ago

I think one pertinent problem may be marking the package.json as module syntax w/ type: 'module' but outputting CommonJS - https://github.com/markerikson/react-prod-sourcemaps/blob/446dd17d335a0f8a69f0af10f89255c7fbfd2b65/tsconfig.json#L8

Since node supports ESM solidly nowadays, maybe we could output ESM instead of CommonJS or outputting both? ex: https://github.com/vitejs/vite/discussions/13928

markerikson commented 1 year ago

oh wait, yeah, why do we have type: "module"? I found that just causes more problems in general - it's better to have the specific filenames instead:

https://blog.isquaredsoftware.com/2023/08/esm-modernization-lessons/

JonasBa commented 1 year ago

I just fixed this - I removed type:module and used tsup to output ESM and CJS builds in https://github.com/markerikson/react-prod-sourcemaps/pull/10

markerikson commented 1 year ago

Updated #10 and it should look good now:

┌───────────────────┬───────────────────────────────────┐
│                   │ "@acemarke/react-prod-sourcemaps" │
├───────────────────┼───────────────────────────────────┤
│ node10            │ 🟢                                │
├───────────────────┼───────────────────────────────────┤
│ node16 (from CJS) │ 🟢 (CJS)                          │
├───────────────────┼───────────────────────────────────┤
│ node16 (from ESM) │ 🟢 (ESM)                          │
├───────────────────┼───────────────────────────────────┤
│ bundler           │ 🟢                                │
└───────────────────┴───────────────────────────────────┘
markerikson commented 1 year ago

Okay, published https://github.com/markerikson/react-prod-sourcemaps/releases/tag/0.1.0 !

jasikpark commented 1 year ago

thx! i'll test today

JonasBa commented 1 year ago

I just tested this for Sentry and it properly remapped our react build. @markerikson do you think there is a chance we could generate sourcemaps for react-dom/profiling bundle as well? There seem to be a lot of folks (including us at Sentry) that use this in production.

jasikpark commented 1 year ago

upgrading to 0.1.0 and i'm nearly there! just hit one more snag:

✓ 2586 modules transformed.
computing gzip size (0)...[react-sourcemaps] __dirname is not defined
✓ built in 7.45s
error during build:
ReferenceError: __dirname is not defined
    at loadExistingReactDOMSourcemap (file:///Users/calebjasik/Git/defined.net/webclient/node_modules/.pnpm/@acemarke+react-prod-sourcemaps@0.1.0/node_modules/@acemarke/react-prod-sourcemaps/lib/index.mjs:195:31)
    at file:///Users/calebjasik/Git/defined.net/webclient/node_modules/.pnpm/@acemarke+react-prod-sourcemaps@0.1.0/node_modules/@acemarke/react-prod-sourcemaps/lib/index.mjs:258:12
    at file:///Users/calebjasik/Git/defined.net/webclient/node_modules/.pnpm/@ampproject+remapping@2.2.1/node_modules/@ampproject/remapping/dist/remapping.mjs:132:27
    at Array.map (<anonymous>)
    at build (file:///Users/calebjasik/Git/defined.net/webclient/node_modules/.pnpm/@ampproject+remapping@2.2.1/node_modules/@ampproject/remapping/dist/remapping.mjs:119:38)
    at buildSourceMapTree (file:///Users/calebjasik/Git/defined.net/webclient/node_modules/.pnpm/@ampproject+remapping@2.2.1/node_modules/@ampproject/remapping/dist/remapping.mjs:110:16)
    at remapping (file:///Users/calebjasik/Git/defined.net/webclient/node_modules/.pnpm/@ampproject+remapping@2.2.1/node_modules/@ampproject/remapping/dist/remapping.mjs:186:18)
    at maybeRewriteSourcemapWithReactProd (file:///Users/calebjasik/Git/defined.net/webclient/node_modules/.pnpm/@acemarke+react-prod-sourcemaps@0.1.0/node_modules/@acemarke/react-prod-sourcemaps/lib/index.mjs:233:20)
    at rewireSourceMapsFromGeneratedAssetList (file:///Users/calebjasik/Git/defined.net/webclient/node_modules/.pnpm/@acemarke+react-prod-sourcemaps@0.1.0/node_modules/@acemarke/react-prod-sourcemaps/lib/index.mjs:31:27)
    at Object.writeBundle (file:///Users/calebjasik/Git/defined.net/webclient/node_modules/.pnpm/@acemarke+react-prod-sourcemaps@0.1.0/node_modules/@acemarke/react-prod-sourcemaps/lib/index.mjs:80:11)
    at file:///Users/calebjasik/Git/defined.net/webclient/node_modules/.pnpm/rollup@3.29.3/node_modules/rollup/dist/es/shared/node-entry.js:25539:40
    at async Promise.all (index 0)
    at async PluginDriver.hookParallel (file:///Users/calebjasik/Git/defined.net/webclient/node_modules/.pnpm/rollup@3.29.3/node_modules/rollup/dist/es/shared/node-entry.js:25467:9)
    at async file:///Users/calebjasik/Git/defined.net/webclient/node_modules/.pnpm/rollup@3.29.3/node_modules/rollup/dist/es/shared/node-entry.js:26787:13
    at async catchUnfinishedHookActions (file:///Users/calebjasik/Git/defined.net/webclient/node_modules/.pnpm/rollup@3.29.3/node_modules/rollup/dist/es/shared/node-entry.js:25905:16)
    at async build (file:///Users/calebjasik/Git/defined.net/webclient/node_modules/.pnpm/vite@5.0.0-beta.1/node_modules/vite/dist/node/chunks/dep-e31699fa.js:48096:22)
    at async CAC.<anonymous> (file:///Users/calebjasik/Git/defined.net/webclient/node_modules/.pnpm/vite@5.0.0-beta.1/node_modules/vite/dist/node/cli.js:842:9)
 ELIFECYCLE  Command failed with exit code 1.

maybe it's b/c i have 

 build: {
      outDir: '.definednet/build',
      // Generate source maps for sentry, but don't expose them to the public
      sourcemap: mode === 'production' ? 'hidden' : true,
    },

set in my Vite config?

where does __dirname come from? all I see is

https://github.com/markerikson/react-prod-sourcemaps/blob/281e1168a88e80fb095246c5513c002c283c1ee3/src/build-plugin.test.mts#L145

and

https://github.com/markerikson/react-prod-sourcemaps/blob/281e1168a88e80fb095246c5513c002c283c1ee3/src/index.mts#L66

markerikson commented 1 year ago

Yeah, __dirname implicitly exists in CJS modules, but not ESM. But import.meta exists in ESM and not CJS.

I have a tentative fix for that in #12 .

jasikpark commented 1 year ago

what's the sourcemap viz tool that you use?

jasikpark commented 1 year ago

ah, https://evanw.github.io/source-map-visualization/

markerikson commented 1 year ago

__dirname issue should be fixed:

https://github.com/markerikson/react-prod-sourcemaps/releases/tag/v0.2.0

jasikpark commented 1 year ago

ayyye yup that was my last issue - i just merged a setup using the Vite plugin w/ a pnpm patch for fixing the __dirname usage