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

Build release bundle with Preconstruct #6

Merged
merged 3 commits into from
Jun 1, 2023
Merged

Build release bundle with Preconstruct #6

merged 3 commits into from
Jun 1, 2023

Conversation

bvaughn
Copy link
Owner

@bvaughn bvaughn commented Jun 1, 2023

  • Build release bundles with Preconstruct (smaller, more readable)
  • Replaced CSS modules (which required special bundler configuration to support) with single styles.css file which can be imported anywhere in the app (even if no bundler is used); this should support more bundlers/frameworks/environments

@vercel
Copy link

vercel bot commented Jun 1, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
use-context-menu ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 1, 2023 1:52pm

@bvaughn
Copy link
Owner Author

bvaughn commented Jun 1, 2023

Hey @Andarist, any pointers? 😄

I've tried using preconstruct init and preconstruct fix to walk me through this but I'm getting build errors:

🎁 info building bundles!
🎁 error use-context-menu SyntaxError: Unexpected token (49:9)
🎁 error use-context-menu     at pp$4.raise (/Users/bvaughn/Documents/git/oss/use-context-menu/node_modules/.pnpm/rollup@2.79.1/node_modules/rollup/dist/shared/rollup.js:19551:13)
🎁 error use-context-menu     at pp$9.unexpected (/Users/bvaughn/Documents/git/oss/use-context-menu/node_modules/.pnpm/rollup@2.79.1/node_modules/rollup/dist/shared/rollup.js:16845:8)
🎁 error use-context-menu     at pp$5.parseExprAtom (/Users/bvaughn/Documents/git/oss/use-context-menu/node_modules/.pnpm/rollup@2.79.1/node_modules/rollup/dist/shared/rollup.js:18926:10)
🎁 error use-context-menu     at pp$5.parseExprSubscripts (/Users/bvaughn/Documents/git/oss/use-context-menu/node_modules/.pnpm/rollup@2.79.1/node_modules/rollup/dist/shared/rollup.js:18718:19)
🎁 error use-context-menu     at pp$5.parseMaybeUnary (/Users/bvaughn/Documents/git/oss/use-context-menu/node_modules/.pnpm/rollup@2.79.1/node_modules/rollup/dist/shared/rollup.js:18684:17)
🎁 error use-context-menu     at pp$5.parseExprOps (/Users/bvaughn/Documents/git/oss/use-context-menu/node_modules/.pnpm/rollup@2.79.1/node_modules/rollup/dist/shared/rollup.js:18611:19)
🎁 error use-context-menu     at pp$5.parseMaybeConditional (/Users/bvaughn/Documents/git/oss/use-context-menu/node_modules/.pnpm/rollup@2.79.1/node_modules/rollup/dist/shared/rollup.js:18594:19)
🎁 error use-context-menu     at pp$5.parseMaybeAssign (/Users/bvaughn/Documents/git/oss/use-context-menu/node_modules/.pnpm/rollup@2.79.1/node_modules/rollup/dist/shared/rollup.js:18561:19)
🎁 error use-context-menu     at pp$5.parseExpression (/Users/bvaughn/Documents/git/oss/use-context-menu/node_modules/.pnpm/rollup@2.79.1/node_modules/rollup/dist/shared/rollup.js:18524:19)
🎁 error use-context-menu     at pp$8.parseReturnStatement (/Users/bvaughn/Documents/git/oss/use-context-menu/node_modules/.pnpm/rollup@2.79.1/node_modules/rollup/dist/shared/rollup.js:17165:31) {
🎁 error use-context-menu   pos: 1084,
🎁 error use-context-menu   loc: Position { line: 49, column: 9 },
🎁 error use-context-menu   raisedAt: 1085,
🎁 error use-context-menu   code: 'PLUGIN_ERROR',
🎁 error use-context-menu   plugin: 'resolve-errors',
🎁 error use-context-menu   hook: 'resolveId',
🎁 error use-context-menu   id: '/Users/bvaughn/Documents/git/oss/use-context-menu/packages/use-context-menu/src/components/ContextMenuItem.tsx',
🎁 error use-context-menu   watchFiles: [
🎁 error use-context-menu     '/Users/bvaughn/Documents/git/oss/use-context-menu/packages/use-context-menu/src/index.ts',
🎁 error use-context-menu     '/Users/bvaughn/Documents/git/oss/use-context-menu/packages/use-context-menu/src/types.ts',
🎁 error use-context-menu     '/Users/bvaughn/Documents/git/oss/use-context-menu/packages/use-context-menu/src/utils/isEventType.ts',
🎁 error use-context-menu     '/Users/bvaughn/Documents/git/oss/use-context-menu/packages/use-context-menu/src/components/ContextMenu.tsx',
🎁 error use-context-menu     '/Users/bvaughn/Documents/git/oss/use-context-menu/packages/use-context-menu/src/components/ContextMenuCategory.tsx',
🎁 error use-context-menu     '/Users/bvaughn/Documents/git/oss/use-context-menu/packages/use-context-menu/src/hooks/useModalDismissSignal.ts',
🎁 error use-context-menu     '/Users/bvaughn/Documents/git/oss/use-context-menu/packages/use-context-menu/src/utils/assert.ts',
🎁 error use-context-menu     '/Users/bvaughn/Documents/git/oss/use-context-menu/packages/use-context-menu/src/components/ContextMenuItem.tsx',
🎁 error use-context-menu     '/Users/bvaughn/Documents/git/oss/use-context-menu/packages/use-context-menu/src/components/ContextMenuDivider.tsx',
🎁 error use-context-menu     '/Users/bvaughn/Documents/git/oss/use-context-menu/packages/use-context-menu/src/hooks/useContextMenu.tsx'
🎁 error use-context-menu   ]
🎁 error use-context-menu }
🎁 info If you want to learn more about the above error, check https://preconstruct.tools/errors
🎁 info If the error is not there and you want to learn more about it, open an issue at https://github.com/preconstruct/preconstruct/issues/new
 ELIFECYCLE  Command failed with exit code 1.

babel.config.js Outdated Show resolved Hide resolved
@Andarist
Copy link
Contributor

Andarist commented Jun 1, 2023

Note to myself - it would really be cool if Preconstruct could somehow guide people to configure JSX/TS correctly. It kinda assumes that people already have this... but nowadays - with esbuild and others on the scene - it's not quite true. We were also discussing ditching Babel altogether and stuff - that's also an option. Or perhaps we should just use simple TS+React presets if we don't find any explicit Babel config? That idea also seems to be promising, this way Babel wouldn't have to "spill" into the project if it's not used anywhere else.

@bvaughn
Copy link
Owner Author

bvaughn commented Jun 1, 2023

I like that idea.

@bvaughn
Copy link
Owner Author

bvaughn commented Jun 1, 2023

Got the package building now, just need to sort out a Parcel regression with the website. Thanks!

babel.config.js Outdated Show resolved Hide resolved
@bvaughn
Copy link
Owner Author

bvaughn commented Jun 1, 2023

I think I must have forgotten something because when I try the new release out in Replay I get:

React is not defined

I seem to recall you adding a config setting for this somewhere in at least one of these PRs. Now I just need to remember which one.

@Andarist
Copy link
Contributor

Andarist commented Jun 1, 2023

That setting was strictly related to UMD builds and I doubt that you are using such in Replay.

This problem that you are facing here:

  • is related to using Babel's React preset without the automatic runtime, you are using the classic one (my bad!) - so React.createElement got used for JSX
  • but you have no React identifier available in those files and TS allows that... because you are using the automatic runtime with it ("jsx": "react-jsx")
  • the easiest fix would be to switch to the automatic runtime in Babel's preset but likely you should adjust your peer dep range on React. In older React versions that is not available beyond the very latest version within each major

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.

2 participants