Skip to content

chore(build): Avoid prebuilding api side, instead use an esbuild plugin#7672

Merged
dac09 merged 31 commits intoredwoodjs:mainfrom
dac09:feat/api-skip-prebuild
Apr 5, 2023
Merged

chore(build): Avoid prebuilding api side, instead use an esbuild plugin#7672
dac09 merged 31 commits intoredwoodjs:mainfrom
dac09:feat/api-skip-prebuild

Conversation

@dac09
Copy link
Copy Markdown
Contributor

@dac09 dac09 commented Feb 22, 2023 •
edited
Loading

What does this PR do?

Removes FS-based prebuild process
This changes the API side build process to "prebuild" files in memory with babel, rather than write to the .redwood/prebuild folder. Instead of prebuilding to file system, then transforming with esbuild again, we now use an esbuild plugin to do the same in memory - eliminating all the unnecessary read and writes to the FS.

We should see the dev server (and build process) speed up, particularly for Windows users

Notes
a. In order to use plugins, I switched to the async esbuild build function. This is likely better for perf anyway!
b. We still prebuild files for our tests. This is still handy to do!
c. Renamed prebuildFile -> transformWithBabel to remove any confusion!
d. I've verified that you can debug the api side by setting breakpoints

@dac09 dac09 added the release:chore This PR is a chore (means nothing for users) label Feb 22, 2023
@dac09 dac09 requested a review from jtoar February 22, 2023 10:43
@replay-io
Copy link
Copy Markdown

replay-io Bot commented Feb 22, 2023 •
edited
Loading

16 replays were recorded for c441253.

image 0 Failed
image 16 Passed
    requireAuth graphql checks 
          ```
    locator.waitFor: Target closed
    =========================== logs ===========================
    waiting for locator('.rw-form-error-title').locator('text=You don\'t have permission to do that') to be visible
    ============================================================
    ```
  </ol>
</details>
<li><a href=https://app.replay.io/recording/0693a394-8248-4f1d-990b-a0e782817cb6>useAuth hook, auth redirects checks</a></li>
<li><a href=https://app.replay.io/recording/ddebf561-81a4-487f-9ff3-3f0f0a0f6d10>Check that a specific blog post is prerendered</a></li>
<li><a href=https://app.replay.io/recording/aa152027-b947-40f2-bcd2-1110820c4911>Check that about is prerendered</a></li>
<li><a href=https://app.replay.io/recording/c46342f3-e9a7-405a-b11b-5bcd559784dc>Check that homepage is prerendered</a></li>
<li><a href=https://app.replay.io/recording/41008c3a-2463-4573-93b0-53bdb80e6333>Check that meta-tags are rendering the correct dynamic data</a></li>
<li><a href=https://app.replay.io/recording/e6a43862-f27a-4977-9fd3-462bd853b4d4>Check that you can navigate from home page to specific blog post</a></li>
<li><a href=https://app.replay.io/recording/97f16d8c-4d82-48f9-9757-13eb0e479400>Waterfall prerendering (nested cells)</a></li>
<li><a href=https://app.replay.io/recording/9dc14ec0-5c14-4329-8843-cac7da0ab3bc>RBAC: Admin user should be able to delete contacts</a></li>
<li><a href=https://app.replay.io/recording/77879ce4-feaf-44c7-8f6c-06ad94fccab8>RBAC: Should not be able to delete contact as non-admin user</a></li>
<li><a href=https://app.replay.io/recording/e0ded03d-05ac-416e-804e-97a815c6d077>Smoke test with dev server</a></li>
<li><a href=https://app.replay.io/recording/467188b6-cafa-4bd7-b652-2a6348a39056>Smoke test with rw serve</a></li>
<li><a href=https://app.replay.io/recording/254581da-efd0-4650-b6d6-fbd1a3d0861d>Loads Cell mocks when Cell is nested in another story</a></li>
<li><a href=https://app.replay.io/recording/17cb2ed7-1298-4ed1-8d32-132628a01f79>Loads Cell Stories</a></li>
<li><a href=https://app.replay.io/recording/3ca15030-0185-45d4-87b3-92740db673df>Loads MDX Stories</a></li>
<li><a href=https://app.replay.io/recording/252a0b91-de03-41eb-a0e3-e8f876a2a436>Mocks current user, and updates UI while dev server is running</a></li>

View test run on Replay ↗︎

jtoar
jtoar reviewed Feb 24, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@jtoar jtoar left a comment

Choose a reason for hiding this comment

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

Looking good @dac09! I was trying to come up with a few more ideas around error reporting when transforming with Babel doesn't work for whatever reason, but don't have any concrete suggestions yet. But submitting this review now to get post the two comments I had

Comment thread packages/internal/src/build/api.ts Outdated
Comment thread packages/internal/src/build/api.ts
dac09 added 5 commits February 24, 2023 15:06
@dac09
Try disabling telemetry on CLI tests
d86822e
@dac09
Merge branch 'main' of github.com:redwoodjs/redwood into feat/api-ski…
ccebbeb 
…p-prebuild

* 'main' of github.com:redwoodjs/redwood:
  chore(deps): update dependency @types/uuid to v9.0.1 (redwoodjs#7680)
  chore(deps): update dependency @replayio/playwright to v0.3.23 (redwoodjs#7677)
  chore(deps): update dependency @npmcli/arborist to v6.2.3 (redwoodjs#7675)
  chore(deps): update dependency @envelop/types to v3.0.2 (redwoodjs#7674)
  chore: add codemod for clerk fix in v4.2.0 (redwoodjs#7676)
  chore(deps): update dependency @clerk/types to v3.28.1 (redwoodjs#7652)
  chore(deps): update dependency @envelop/testing to v5.0.6 (redwoodjs#7673)
  Update directives.md (redwoodjs#7670)
  fix(deps): update dependency vscode-languageserver-types to v3.17.3 (redwoodjs#7636)
  Fix `yarn rw exec` to set nonzero exit code on error (redwoodjs#7660)
@dac09
Try mocking telemetry
f2ebf21
@dac09
Add "." to esbuild filter
113ce60
@dac09
Remove prebuild
dc03ee1
@dac09 dac09 closed this Feb 24, 2023
@dac09 dac09 reopened this Feb 24, 2023
dac09
dac09 commented Feb 24, 2023
View reviewed changes
Comment thread packages/cli/src/commands/deploy/__tests__/nftPack.test.js Outdated
@dac09 dac09 marked this pull request as draft March 1, 2023 05:41
@dac09
Copy link
Copy Markdown
Contributor Author

dac09 commented Mar 1, 2023 •
edited
Loading

Todo

  • Look into whether builds are ok on Windows
  • Refactor nftPack tests to avoid actually building the API side

@dac09 dac09 mentioned this pull request Mar 21, 2023
Merged
@Tobbe Tobbe marked this pull request as ready for review April 1, 2023 04:27
@dac09 dac09 merged commit af7a998 into redwoodjs:main Apr 5, 2023
@redwoodjs-bot redwoodjs-bot Bot added this to the next-release milestone Apr 5, 2023
@dac09 dac09 deleted the feat/api-skip-prebuild branch April 5, 2023 06:49
jaiakt pushed a commit to jaiakt/redwood that referenced this pull request Apr 5, 2023
@dac09 @Tobbe
@thedavidprice @jaiakt
chore(build): Avoid prebuilding api side, instead use an esbuild plug…
cf78c93 
…in (redwoodjs#7672)

Co-authored-by: Tobbe Lundberg <[email protected]>
Co-authored-by: David Price <[email protected]>
dac09 added a commit to dac09/redwood that referenced this pull request Apr 6, 2023
@dac09
Merge branch 'feat/vite-ssr' of github.com:dac09/redwood into feat/vi…
c2e6c8b 
…te-ssr

* 'feat/vite-ssr' of github.com:dac09/redwood:
  chore(build): Avoid prebuilding api side, instead use an esbuild plugin (redwoodjs#7672)
  chore(deps): update dependency @clerk/clerk-react to v4.14.2 (redwoodjs#8016)
  chore(deps): update dependency @azure/msal-browser to v2.35.0 (redwoodjs#8009)
  chore(deps): update dependency @supabase/supabase-js to v2.14.0 (redwoodjs#8010)
  fix(deps): update typescript-eslint monorepo to v5.57.1 (redwoodjs#8008)
  fix(deps): update dependency core-js to v3.30.0 (redwoodjs#8007)
  chore(deps): update dependency @playwright/test to v1.32.2 (redwoodjs#8005)
  fix(deps): update dependency @fastify/static to v6.10.0 (redwoodjs#7999)
  chore(deps): update dependency dependency-cruiser to v12.11.1 (redwoodjs#7998)
  Update docs to reference creating an ED25519 key instead of RSA (redwoodjs#8013)
  chore(deps): update dependency @types/react to v18.0.33 (redwoodjs#7996)
  chore(deps): update dependency @replayio/playwright to v0.3.28 (redwoodjs#8006)
  chore(deps): update dependency esbuild to v0.17.15 (redwoodjs#7994)
  chore(deps): update mheap/github-action-required-labels action to v4 (redwoodjs#7997)
  chore(ci): Update node version for telemetry check (redwoodjs#8004)
@jtoar jtoar modified the milestones: next-release, v5.0.0 Apr 11, 2023
jtoar added a commit that referenced this pull request Apr 18, 2023
@jtoar
Revert "chore(build): Avoid prebuilding api side, instead use an esbu…
ba78153 
…ild plugin (#7672)"

This reverts commit af7a998.
jtoar added a commit that referenced this pull request Apr 19, 2023
@jtoar
Revert "chore(build): Avoid prebuilding api side, instead use an esbu…
e9d63f6 
…ild plugin" (#8056)

* Revert "chore(build): Avoid prebuilding api side, instead use an esbuild plugin (#7672)"

This reverts commit af7a998.

* add back chore changes
@jtoar jtoar mentioned this pull request Apr 20, 2023
Closed
@jtoar jtoar modified the milestones: v5.0.0, chore Apr 27, 2023
@dac09 dac09 mentioned this pull request Dec 28, 2023
Merged
jtoar added a commit that referenced this pull request Jan 18, 2024
@dac09 @Tobbe @jtoar
chore(build): Avoid prebuilding api side, instead use an esbuild plug…
4f1cd09 
…in (#9767)

Resurrecting this again, to see if I can get CI more stable this time 

**2nd edition Implementation:** With the second edition, I'm trying
esbuild's watcher - rather than chokidar
This is a no go ❌, because when new files are introduced the watcher
isn't triggered.

**3rd edition implementation**: Use esbuild's `rebuild` function that
_may_ provide performance benefits

Old PR #7672

---------

Co-authored-by: Tobbe Lundberg <[email protected]>
Co-authored-by: Dominic Saadi <[email protected]>
@Josh-Walker-GM Josh-Walker-GM modified the milestones: chore, v8.0.0 Sep 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@jtoar jtoar jtoar left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

release:chore This PR is a chore (means nothing for users)

Projects

None yet

Milestone

v8.0.0

Development

Successfully merging this pull request may close these issues.

5 participants

@dac09 @jtoar @Josh-Walker-GM @Tobbe @thedavidprice