Skip to content
This repository has been archived by the owner on Dec 5, 2024. It is now read-only.

feat: add useAsync and improve ssrRef #28

Merged
merged 25 commits into from
May 8, 2020
Merged

feat: add useAsync and improve ssrRef #28

merged 25 commits into from
May 8, 2020

Conversation

mathe42
Copy link
Collaborator

@mathe42 mathe42 commented May 5, 2020

Features

  • add useAsync helper to prefetch data and send in ssrContext to client
  • updates serverPrefetch helper
  • improves ssrRef handling

@mathe42 mathe42 marked this pull request as draft May 5, 2020 14:48
@mathe42 mathe42 marked this pull request as ready for review May 5, 2020 18:32
@mathe42
Copy link
Collaborator Author

mathe42 commented May 5, 2020

When this PR is accepted we have to rewrite fetch. I didn't look into that :D

Fell free to discouse about this. I don't know if my way is the best way...

@mathe42 mathe42 changed the title WIP: (feat) namespace (feat) namespace May 5, 2020
@danielroe danielroe added the enhancement New feature or request label May 6, 2020
@mathe42
Copy link
Collaborator Author

mathe42 commented May 6, 2020

Finished for review.

Copy link
Collaborator Author

@mathe42 mathe42 left a comment

Choose a reason for hiding this comment

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

@danielroe changed ssrRef to only hydrate if value is function or value of ref get changed

src/fetch.ts Outdated Show resolved Hide resolved
src/async.ts Show resolved Hide resolved
src/server-prefetch.ts Show resolved Hide resolved
test/fixture/pages/ssr-ref.vue Show resolved Hide resolved
useasync cannot use router hoocks when setup is called routerGuards are finished
@mathe42 mathe42 changed the title (feat) namespace (feat) rewrite ssrRef May 7, 2020
@mathe42
Copy link
Collaborator Author

mathe42 commented May 7, 2020

Note: The old and my new implementation do not allow to use outside of setup! I'm working on that there are some problems with watch. vuejs/composition-api#311 might fix them.

@mathe42 mathe42 mentioned this pull request May 7, 2020
Copy link
Member

@danielroe danielroe left a comment

Choose a reason for hiding this comment

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

Looking really interesting. Apart from the comments below, would you consider adding some e2e tests (or possibly we'll need to do it in jest if we're inspecting the raw HTML for the __NUXT__ content). I'd be happy to do the latter if you'd prefer, after it gets merged.

src/fetch.ts Outdated Show resolved Hide resolved
src/server-prefetch.ts Outdated Show resolved Hide resolved
src/ssr-ref.ts Outdated Show resolved Hide resolved
src/ssr-ref.ts Outdated Show resolved Hide resolved
test/fixture/pages/ssr-ref.vue Show resolved Hide resolved
test/fixture/pages/ssr-ref.vue Outdated Show resolved Hide resolved
src/ssr-ref.ts Outdated Show resolved Hide resolved
src/async.ts Outdated Show resolved Hide resolved
src/async.ts Show resolved Hide resolved
src/ssr-ref.ts Outdated Show resolved Hide resolved
@mathe42 mathe42 requested a review from danielroe May 7, 2020 21:07
src/ssr-ref.ts Outdated Show resolved Hide resolved
src/babel.ts Outdated Show resolved Hide resolved
Co-authored-by: Daniel Roe <[email protected]>
src/ssr-ref.ts Outdated Show resolved Hide resolved
src/async.ts Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@mathe42 mathe42 requested a review from danielroe May 8, 2020 07:32
@mathe42
Copy link
Collaborator Author

mathe42 commented May 8, 2020

I don't know where the test fails. Lokaly node test/start-fixture.js doesn't work anymore:

ERROR  Failed to compile with 1 errors                                                                         friendly-errors 09:22:20


 ERROR  in ./lib/index.es.js                                                                                    friendly-errors 09:22:20

Cannot statically analyse 'require(…, …)' in line 461                                                           friendly-errors 09:22:20
                                                                                                                friendly-errors 09:22:20
 @ ./test/fixture/.nuxt/composition-api/plugin.js 7:0-53
 @ ./test/fixture/.nuxt/index.js
 @ ./test/fixture/.nuxt/client.js
 @ multi eventsource-polyfill webpack-hot-middleware/client?reload=true&timeout=30000&ansiColors=&overlayStyles=&name=client&path=/__webpack_hmr/client ./test/fixture/.nuxt/client.js

src/babel.ts Outdated Show resolved Hide resolved
@danielroe
Copy link
Member

I don't know where the test fails. Lokaly node test/start-fixture.js doesn't work anymore:

ERROR  Failed to compile with 1 errors                                                                         friendly-errors 09:22:20


 ERROR  in ./lib/index.es.js                                                                                    friendly-errors 09:22:20

Cannot statically analyse 'require(…, …)' in line 461                                                           friendly-errors 09:22:20
                                                                                                                friendly-errors 09:22:20
 @ ./test/fixture/.nuxt/composition-api/plugin.js 7:0-53
 @ ./test/fixture/.nuxt/index.js
 @ ./test/fixture/.nuxt/client.js
 @ multi eventsource-polyfill webpack-hot-middleware/client?reload=true&timeout=30000&ansiColors=&overlayStyles=&name=client&path=/__webpack_hmr/client ./test/fixture/.nuxt/client.js

You're getting the error because the change to the Babel plugin is now injecting a key into every function call, including (in this case) require.

danielroe added a commit that referenced this pull request May 8, 2020
adopted partial approach from #28
@mathe42
Copy link
Collaborator Author

mathe42 commented May 8, 2020

Why are these test failing?

@danielroe
Copy link
Member

danielroe commented May 8, 2020

Why are these test failing?

The reason they are now failing is that the e2e tests now run against a built version of the fixture, where this error is occuring: Node.appendChild: Cannot add children to a Comment. The error is caused by the current implementation of onServerPrefetch.

@mathe42
Copy link
Collaborator Author

mathe42 commented May 8, 2020

this works:

<template>
  <div>
    <div>test: {{test}}</div>
  </div>
</template>

<script>
import {
  defineComponent,
  ref,
  onServerPrefetch
} from '@vue/composition-api'

export default defineComponent({
  setup() {
    const test = ref('test')

    onServerPrefetch(() => {
      test.value = 'Hello World.'
    })

    return {
      test
    }
  },
})
</script>

this doesn't work:

<template>
  <div>
    <div>test: {{test}}</div>
  </div>
</template>

<script>
import {
  defineComponent,
  ref,
  onServerPrefetch
} from 'nuxt-composition-api'

export default defineComponent({
  setup() {
    const test = ref('test')

    onServerPrefetch(() => {
      test.value = 'Hello World.'
    })

    return {
      test
    }
  },
})
</script>

@mathe42
Copy link
Collaborator Author

mathe42 commented May 8, 2020

@danielroe had to rewrite onServerPrefetch now it works.

src/ssr-ref.ts Outdated Show resolved Hide resolved
src/ssr-ref.ts Outdated Show resolved Hide resolved
@mathe42
Copy link
Collaborator Author

mathe42 commented May 8, 2020

That was wanted when useing a function I think it should be in __NUXT__

src/ssr-ref.ts Outdated Show resolved Hide resolved
@mathe42
Copy link
Collaborator Author

mathe42 commented May 8, 2020

Looks good for me.

@danielroe danielroe changed the title (feat) rewrite ssrRef feat: add useAsync and improve ssrRef May 8, 2020
@danielroe danielroe merged commit 31c9729 into nuxt-community:master May 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants