Redwood v6.0.0 Upgrade Guide

Announcements Releases and Upgrade Guides
45

renaming my ‘subscriptions’ query to ‘fetchSubscriptions’ worked for me which looks like your recommended approach as well.

46

We’re seeing a similar issue with mocking context.currentUser on 6.0.3 in a test using mockRedwoodDirective.

This is our directive:

import {
  createTransformerDirective,
  TransformerDirectiveFunc,
} from '@redwoodjs/graphql-server'

import { sendToAll } from 'src/functions/webSockets'
import { getOrgId } from 'src/lib/customAuth'
import { logger } from 'src/lib/logger'

export const schema = gql`
  """
  Use @notifySubscribers to notify subscriber that data is changed.
  This transformer doesn't transform anything. Due to Redwood support only two types of
  directives - transformers and validators - transformer is used here as just a middleware.
  """
  directive @notifySubscribers on FIELD_DEFINITION
`

const transform: TransformerDirectiveFunc = async ({
  context,
  resolvedValue,
}) => {
  // currentUser is only available when auth is setup.
  logger.debug(
    { currentUser: context.currentUser },
    'currentUser in notifySubscribers directive'
  )

  // ... you can modify the resolvedValue and return it
  logger.debug(resolvedValue, 'resolvedValue in notifySubscribers directive')

  try {
    await sendToAll(JSON.stringify(resolvedValue), getOrgId())
  } catch (e) {
    logger.error('error object:', e)
  }

  return resolvedValue
}

const notifySubscribers = createTransformerDirective(schema, transform)

export default notifySubscribers

This test used to work in v5:

describe('notifySubscribers directive', () => {
  it('declares the directive sdl as schema, with the correct name', () => {
    expect(notifySubscribers.schema).toBeTruthy()
    expect(getDirectiveName(notifySubscribers.schema)).toBe('notifySubscribers')
  })

  it('has a notifySubscribers implementation to not transforms the value', async () => {
    const mockExecution = mockRedwoodDirective(notifySubscribers, {
      mockedResolvedValue: 'foo',
    })

    expect(await mockExecution()).toBe('foo')
  })
})

but now fails because context is undefined in the directive:

    TypeError: Cannot read properties of undefined (reading 'currentUser')

      23 |   // currentUser is only available when auth is setup.
      24 |   logger.debug(
    > 25 |     { currentUser: context.currentUser },
         |                            ^
      26 |     'currentUser in notifySubscribers directive'
      27 |   )
      28 |

I tried passing in context a couple of different ways:

  it('has a notifySubscribers implementation to not transforms the value', async () => {
    const mockExecution = mockRedwoodDirective(notifySubscribers, {
      context: {
        currentUser: {
          email: 'test@test.com',
        },
      },
      mockedResolvedValue: 'foo',
    })

    expect(await mockExecution()).toBe('foo')
  })

describe('notifySubscribers directive', () => {
  it('declares the directive sdl as schema, with the correct name', () => {
    expect(notifySubscribers.schema).toBeTruthy()
    expect(getDirectiveName(notifySubscribers.schema)).toBe('notifySubscribers')
  })

  it('has a notifySubscribers implementation to not transforms the value', async () => {
    mockCurrentUser({
      email: 'test@test.com',
    })
    const mockExecution = mockRedwoodDirective(notifySubscribers, {
      mockedResolvedValue: 'foo',
    })

    expect(await mockExecution()).toBe('foo')
  })
})

but get the same error. Any advice?

1 Like
47

Apologies if this is answered elsewhere and I’ve missed it. I looked through this thread and the v6.0.0 announcement thread as well and did not find it.

Is it required to upgrade to v5 before v6, or can we upgrade directly from v4 to v6? I’ve been putting off updating to v5 and would rather just jump to v6 if that’s possible.

48

I’d recommend upgrading to v5 first due to the changes to React 18 and Typescript — and then testing locally that your app behaves as expected.

Then upgrade to v6.

1 Like
49

Hey thank you for grate work for your migration guide.

I’d share where I got stacked.
My previous web/src/index.html was like this

<!DOCTYPE html>
<html lang="en">

<head>
  <meta charset="UTF-8" />
  <meta name="viewport" content="width=device-width, initial-scale=1.0" />
  <link rel="icon" type="image/x-icon" href="/favicon.ico" />
</head>

<body>
  <div id="redwood-app">
    <!-- Please keep the line below for prerender support. -->
    <%= prerenderPlaceholder %>
  </div>
</body>

</html>

This got an error message below.

6:17:27 PM [vite] Internal server error: Unable to parse HTML; parse5 error code invalid-first-character-of-tag-name
web | at /xxx/web/src/index.html:14:6
web | 12 |


web | 13 |
web | 14 | <%= prerenderPlaceholder %>
web | | ^
web | 15 |

web | 16 |
web | at xxx/web/src/index.html:14:6

Then I replaced my index.html file with below and the error is gone.

50

This was in the v5.0.0 Upgrade Guide and necessary for the React 18 upgrade.

2 Likes
51

One thing you can try is switching from:

beforeEach(jest.resetAllMocks)

to:

afterEach(jest.resetAllMocks)

which worked for me when I was debugging your example. I think this is a reasonable change but let me know what you think and if that has other consequences for your tests? We now internally setup a mock to handle mocking context isolation so if you reset all mocks before the tests are run you’ll reset that one too.

52

Hey @asher-eastham, thanks for finding this one! This does require a small patch on our end so I’ll aim to try and get that out as soon as possible.

Looks like we’ll support both methods of using context in the directive mocking:

  • Passing in an explicit context as parameter to the mockRedwoodDirective function
  • Using mockCurrentUser to set some context and then using mockRedwoodDirective without passing in an explicit context`

Your v5 version of the test should still work too as we should always be passing a context object not an undefined. I’ll update when a patch is out to hopefully confirm that it fixes things for you.

1 Like
53

@noire.munich and @Entelechy, I just released a patch that allows Prisma models and GraphQL types with the “Subscription” name, so if you upgrade to v6.0.4 you shouldn’t have to rename your model, though we still recommend renaming it because it’ll conflict when realtime comes out.

1 Like
55

@asher-eastham v6.0.5 was just released which should fix the issue you ran into. Let me know if it fixes the issue or if there is more to investigate when you get the chance to try it out.

1 Like
56

Hi, team. I would like to report a small breakage in my repo. I have the following function that gets called by all the non-graphql functions to set up “context” just like we would have for graphQL, as we tend to share methods that look into the context.

export function initContextForNonGraphQL(request: APIGatewayEvent): void {
  // The following line binds the asyncLocalStorage to the current context
  // If we don't do this, the context (which is a proxy over the asyncLocalStorage) will return undefined
  // This already gets done for us for graphql, but not for non-graphql endpoints
  // We need asyncLocalStorage since we are serverful and need to keep request contexts separate
  // Redwood will fall back to a global in-memory context for non-graphql serverless functions which use one process per request
  getAsyncStoreInstance().enterWith(new Map())

  const formalizeSpecificContextVars = getFormalizeSpecificContextVars(request)
  logger.debug('setting serverful context', formalizeSpecificContextVars)
  setContext(formalizeSpecificContextVars)
}

This is now breaking under testing because in @redwoodjs/testing/config/jest/api/jest.setup.js getAsyncStoreInstance is mocked without enterWith . What is the recommended way to fix this?

EDIT: Currently bypassing that line by checking NODE_ENV !== 'test'

2 Likes
57

Hey @xmonkee, thanks! Can I just double check this is code you’re calling in redwood functions? You needed to do this because in v5 if you didn’t your global context would simply be a global object which would then be shared across different requests which called these functions?

My first impression is that we should be doing this for you internally so that context is isolated for functions just as it is for graphql. We want your context isolated for all requests regardless of what they happen to be doing.

I can confirm your code as is won’t work in v6. As you say we don’t include the enterWith in the mocked object. This may be something I have to fix but I would like to take a further look tomorrow to check if we’re doing this context isolation internally so you wouldn’t need to do this yourself.

58

That fixed it thank you!

2 Likes
59

Okay we got v6 deployed to production, but noticed our API was incredibly sluggish. I’m talking minimum 4.5 seconds for an API request. 20x more than our normal.

I reverted back to the version we had prior and it was completely normal again.
I’m unsure what data you’re going to need to be able to help us rectify this, but that’s a big blocker.

60

Hey @jonoc330, glad context in testing is working for you, but the sluggish API is definitely concerning and something we’re keen on figuring out. We’ll think about what data we need, and try to reproduce it with what we have. Something that may help right now: were you running a v5 release in production before upgrading to v6? That is, what your comparing the slow down to is a v5 release? Thanks!

61

Hey, @Josh-Walker-GM , yeah, i’m calling this in redwood. All my redwood functions begin like this:

export const handler: Handler<APIGatewayEvent> = async (event, context, callback) => {
  initContextForNonGraphQL(event)
  ...

Yeah, I needed to do this to isolate the context in a serverful deployment for non-graphql functions. I am actually not sure if you can even call “setContext” without doing this? But yeah, without this, there is a pretty big risk that context will leak across requests in non-graphql functions, and if redwood can take care of that, that would be awesome.

1 Like
62

Thanks, it looks like those test failures are resolved! I have a few more to look into, will post again if it seems to be a framework issue.

2 Likes
63

We are also experiencing test failures with similar reason of:

This is now breaking under testing because in @redwoodjs/testing/config/jest/api/jest.setup.js getAsyncStoreInstance is mocked without enterWith

We use serverless and to have the user in other functions, we do:

export const handler = useRequireAuth({
   handlerFn: myHandlerFunction,
   getCurrentUser,
})

While executing tests, we got:

typeerror: (0 , _globalcontextstore.getasyncstoreinstance)(...).run is not a function
3 Likes
64

@jonoc330 we’ve identified at least one of the performance concerns and released a patch. Could you try with the v6.0.6 patch when you have the chance? We’ll keep working on this in the meantime, thanks!

65

Hey there,

Going through the upgrade process now and logging out now longer works properly. It keeps recursively calling one of my routes and gets stuck in an infinite loop. This has never happened before the upgrade.

Did anything change with how the Set and Private components handle the unauthenticated props by chance? That’s the only thing I can think of that would be causing this. I think I could probably get around it for now by navigating directly to the login page after calling logOut, but seems like something weird is going on so wanted to mention. Otherwise the guide has been great, thanks!