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

chore: remove api side polyfilling #7712

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Conversation

jtoar
Copy link
Contributor

@jtoar jtoar commented Mar 1, 2023

Note: I need to figure out the dependency situation (@babel/runtime vs @babel/runtime-corejs3, and where they should go), but I think this is the minimal set of config changes to remove polyfills for the api side. I'm happy to make more Babel changes because there's still a bit we can do to clean up our config, but one thing at a time.

What this doesn't do:

  • anything to the web side
  • anything to redwood packages

It only affects yarn rw build api. So how a redwood app's api side is built. The change here is: we no longer polyfill anything.

The reasoning: the api side targets a fairly predictable runtime, Node.js (and right now only two versions of it). We don't need to polyfill en-masse (and we were—we were polyfilling things that didn't need to be, like Array.includes, forEach, etc.). Moreover, what we do polyfill that's not in the versions of Node we target is esoteric and 99% proposals (see the test here). Many of those proposals are in Stage 1, Stage 0, or worse, were rejected.

@jtoar jtoar added the release:breaking This PR is a breaking change label Mar 1, 2023
@jtoar jtoar requested a review from dac09 March 1, 2023 03:04
@replay-io
Copy link

replay-io bot commented Mar 1, 2023

16 replays were recorded for 0cd18ec.

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/ba982ba2-1786-4d4a-8dee-990d6666af54>useAuth hook, auth redirects checks</a></li>
      <li><a href=https://app.replay.io/recording/75ddbe0d-9b59-46d4-bf01-72f08893d875>Check that <meta> tags are rendering the correct dynamic data</a></li>
      <li><a href=https://app.replay.io/recording/7c5a47f1-214e-4991-a5c6-9c213ba8adb2>Check that a specific blog post is prerendered</a></li>
      <li><a href=https://app.replay.io/recording/6bb0837a-cae9-4190-84e3-a1638d0cd2b5>Check that about is prerendered</a></li>
      <li><a href=https://app.replay.io/recording/f11fa618-5759-4713-98ba-8eea2cdc11c9>Check that homepage is prerendered</a></li>
      <li><a href=https://app.replay.io/recording/d20fb12d-232b-4105-91d8-3539c64dba5d>Check that you can navigate from home page to specific blog post</a></li>
      <li><a href=https://app.replay.io/recording/be2dcb79-cd86-4a3e-b818-7f7975e091d4>Waterfall prerendering (nested cells)</a></li>
      <li><a href=https://app.replay.io/recording/8c8b3554-cddf-4c9d-91e2-aba42df81277>RBAC: Admin user should be able to delete contacts</a></li>
      <li><a href=https://app.replay.io/recording/126f9f26-cf34-4f9d-8b5f-d57ee3d52aa4>RBAC: Should not be able to delete contact as non-admin user</a></li>
      <li><a href=https://app.replay.io/recording/7d7b420b-34db-4089-808a-705c92399ced>Smoke test with dev server</a></li>
      <li><a href=https://app.replay.io/recording/4f7765b5-96f4-4d2a-8a5e-a967af002409>Smoke test with rw serve</a></li>
      <li><a href=https://app.replay.io/recording/9778abb4-2874-4ba9-b99a-84faadd9bd3c>Loads Cell mocks when Cell is nested in another story</a></li>
      <li><a href=https://app.replay.io/recording/28dda8cc-43ea-4952-9c98-83de98da0028>Loads Cell Stories</a></li>
      <li><a href=https://app.replay.io/recording/b44967e5-423c-4308-b804-1d714ebc7ca6>Loads MDX Stories</a></li>
      <li><a href=https://app.replay.io/recording/102f466f-705f-4932-a485-3f671a4e81db>Mocks current user, and updates UI while dev server is running</a></li>
      

View test run on Replay ↗︎

@dac09
Copy link
Collaborator

dac09 commented Mar 1, 2023

I haven't looked at this properly yet Dom, just the description, but worth keeping in mind that if we have polyfills in the framework, runtime is going to have to be a dependency.

I would say let's remove the polyfills from the framework too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:breaking This PR is a breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants