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

fix(intergration defined middleware): prevent importing non-existent user middleware #9053

Closed

Conversation

lilnasy
Copy link
Contributor

@lilnasy lilnasy commented Nov 10, 2023

Changes

Testing

new fixture

Docs

Doesn't affect usage

Copy link

changeset-bot bot commented Nov 10, 2023

🦋 Changeset detected

Latest commit: bddfd68

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot added the pkg: astro Related to the core `astro` package (scope) label Nov 10, 2023
@github-actions github-actions bot added the pr: docs A PR that includes documentation for review label Nov 10, 2023
Copy link
Contributor

@matthewp matthewp left a comment

Choose a reason for hiding this comment

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

Thanks!

Copy link
Member

@MoustaphaDev MoustaphaDev left a comment

Choose a reason for hiding this comment

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

Nice refactor!

@lilnasy
Copy link
Contributor Author

lilnasy commented Nov 10, 2023

Do not merge until green.
The tests are not right, but the implementation seems correct.

Copy link
Member

@ematipico ematipico left a comment

Choose a reason for hiding this comment

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

The issue is at line 47. If the user doesn't have a middleware, we bail and we emit export const onRequest = undefined.

In case the user doesn't have a middleware, we have to create a new one from scratch and not attempt to import the one from the user.

This isn't the correct fix. At like 47 you also need to check if there are integration adapters.

@lilnasy
Copy link
Contributor Author

lilnasy commented Nov 10, 2023

@ematipico could you take it from here?

@ematipico ematipico self-assigned this Nov 10, 2023
@lilnasy
Copy link
Contributor Author

lilnasy commented Nov 10, 2023

!preview

Copy link
Contributor

Invalid command. Expected: "/preview "

@lilnasy
Copy link
Contributor Author

lilnasy commented Nov 10, 2023

/preview

Copy link
Contributor

Invalid command. Expected: "/preview "

@lilnasy
Copy link
Contributor Author

lilnasy commented Nov 10, 2023

/preview name

Copy link
Contributor

Snapshots have been released for the following packages:

  • astro@experimental--name
Publish Log
🦋  warn ===============================IMPORTANT!===============================
🦋  warn Packages will be released under the experimental--name tag
🦋  warn ----------------------------------------------------------------------
🦋  info npm info astro
🦋  info npm info @astrojs/prism
🦋  info npm info @astrojs/rss
🦋  info npm info create-astro
🦋  info npm info @astrojs/alpinejs
🦋  info npm info @astrojs/lit
🦋  info npm info @astrojs/markdoc
🦋  info npm info @astrojs/mdx
🦋  info npm info @astrojs/node
🦋  info npm info @astrojs/partytown
🦋  info npm info @astrojs/preact
🦋  info npm info @astrojs/prefetch
🦋  info npm info @astrojs/react
🦋  info npm info @astrojs/sitemap
🦋  info npm info @astrojs/solid-js
🦋  info npm info @astrojs/svelte
🦋  info npm info @astrojs/tailwind
🦋  info npm info @astrojs/vercel
🦋  info npm info @astrojs/vue
🦋  info npm info @astrojs/internal-helpers
🦋  info npm info @astrojs/markdown-remark
🦋  info npm info @astrojs/telemetry
🦋  info npm info @astrojs/underscore-redirects
🦋  info astro is being published because our local version (0.0.0-name-20231110143304) has not been published on npm
🦋  warn @astrojs/prism is not being published because version 3.0.0 is already published on npm
🦋  warn @astrojs/rss is not being published because version 3.0.0 is already published on npm
🦋  warn create-astro is not being published because version 4.5.0 is already published on npm
🦋  warn @astrojs/alpinejs is not being published because version 0.3.1 is already published on npm
🦋  warn @astrojs/lit is not being published because version 3.0.3 is already published on npm
🦋  warn @astrojs/markdoc is not being published because version 0.7.1 is already published on npm
🦋  warn @astrojs/mdx is not being published because version 1.1.4 is already published on npm
🦋  warn @astrojs/node is not being published because version 6.0.3 is already published on npm
🦋  warn @astrojs/partytown is not being published because version 2.0.2 is already published on npm
🦋  warn @astrojs/preact is not being published because version 3.0.1 is already published on npm
🦋  warn @astrojs/prefetch is not being published because version 0.4.1 is already published on npm
🦋  warn @astrojs/react is not being published because version 3.0.4 is already published on npm
🦋  warn @astrojs/sitemap is not being published because version 3.0.3 is already published on npm
🦋  warn @astrojs/solid-js is not being published because version 3.0.2 is already published on npm
🦋  warn @astrojs/svelte is not being published because version 4.0.3 is already published on npm
🦋  warn @astrojs/tailwind is not being published because version 5.0.2 is already published on npm
🦋  warn @astrojs/vercel is not being published because version 5.2.0 is already published on npm
🦋  warn @astrojs/vue is not being published because version 3.0.4 is already published on npm
🦋  warn @astrojs/internal-helpers is not being published because version 0.2.1 is already published on npm
🦋  warn @astrojs/markdown-remark is not being published because version 3.4.0 is already published on npm
🦋  warn @astrojs/telemetry is not being published because version 3.0.4 is already published on npm
🦋  warn @astrojs/underscore-redirects is not being published because version 0.3.3 is already published on npm
🦋  info Publishing "astro" at "0.0.0-name-20231110143304"
🦋  success packages published successfully:
🦋  astro@0.0.0-name-20231110143304
🦋  Creating git tag...
🦋  New tag:  astro@0.0.0-name-20231110143304
Build Log

> root@0.0.0 build /home/runner/work/astro/astro
> turbo run build --filter=astro --filter=create-astro --filter="@astrojs/*" --filter="@benchmark/*"

• Packages in scope: @astrojs/alpinejs, @astrojs/cloudflare, @astrojs/internal-helpers, @astrojs/lit, @astrojs/markdoc, @astrojs/markdown-remark, @astrojs/mdx, @astrojs/netlify, @astrojs/node, @astrojs/partytown, @astrojs/preact, @astrojs/prefetch, @astrojs/prism, @astrojs/react, @astrojs/rss, @astrojs/sitemap, @astrojs/solid-js, @astrojs/svelte, @astrojs/tailwind, @astrojs/telemetry, @astrojs/underscore-redirects, @astrojs/vercel, @astrojs/vue, @benchmark/timer, astro, create-astro
• Running build in 26 packages
• Remote caching enabled
::group::create-astro:build
cache hit, suppressing logs 09c5b8d815f0201d
::endgroup::
::group::@astrojs/telemetry:build
cache miss, executing 2e0085a3082188a3

> @astrojs/telemetry@3.0.4 build /home/runner/work/astro/astro/packages/telemetry
> astro-scripts build "src/**/*.ts" && tsc

::endgroup::
::group::@astrojs/internal-helpers:build
cache miss, executing 5c76356420e50355

> @astrojs/internal-helpers@0.2.1 build /home/runner/work/astro/astro/packages/internal-helpers
> astro-scripts build "src/**/*.ts" && tsc -p tsconfig.json

::endgroup::
::group::@astrojs/prism:build
cache miss, executing b021aa7133667a19

> @astrojs/prism@3.0.0 build /home/runner/work/astro/astro/packages/astro-prism
> astro-scripts build "src/**/*.ts" && tsc -p ./tsconfig.json

::endgroup::
::group::@astrojs/markdown-remark:build
cache miss, executing 5dda81d6e76fa778

> @astrojs/markdown-remark@3.4.0 build /home/runner/work/astro/astro/packages/markdown/remark
> astro-scripts build "src/**/*.ts" && tsc -p tsconfig.json

::endgroup::
::group::astro:build
cache miss, executing 1ca74de4eb4fd2f6

> astro@0.0.0-name-20231110143304 build /home/runner/work/astro/astro/packages/astro
> pnpm run prebuild && astro-scripts build "src/**/*.{ts,js}" && tsc && pnpm run postbuild


> astro@0.0.0-name-20231110143304 prebuild /home/runner/work/astro/astro/packages/astro
> astro-scripts prebuild --to-string "src/runtime/server/astro-island.ts" "src/runtime/client/{idle,load,media,only,visible}.ts"


> astro@0.0.0-name-20231110143304 postbuild /home/runner/work/astro/astro/packages/astro
> astro-scripts copy "src/**/*.astro" && astro-scripts copy "src/**/*.wasm"

::endgroup::
::group::@astrojs/underscore-redirects:build
cache miss, executing 54c327f2c9e986a0

> @astrojs/underscore-redirects@0.3.3 build /home/runner/work/astro/astro/packages/underscore-redirects
> astro-scripts build "src/**/*.ts" && tsc -p tsconfig.json

::endgroup::
::group::@benchmark/timer:build
cache miss, executing 0db9a083e20151cb

> @benchmark/timer@0.0.0 build /home/runner/work/astro/astro/benchmark/packages/timer
> astro-scripts build "src/**/*.ts" && tsc

::endgroup::
::group::@astrojs/partytown:build
cache miss, executing 190bdc1e024eec06

> @astrojs/partytown@2.0.2 build /home/runner/work/astro/astro/packages/integrations/partytown
> astro-scripts build "src/**/*.ts" && tsc

::endgroup::
::group::@astrojs/rss:build
cache miss, executing 5d2333125bee54a6

> @astrojs/rss@3.0.0 build /home/runner/work/astro/astro/packages/astro-rss
> astro-scripts build "src/**/*.ts" && tsc

::endgroup::
::group::@astrojs/preact:build
cache miss, executing fec2844b47280346

> @astrojs/preact@3.0.1 build /home/runner/work/astro/astro/packages/integrations/preact
> astro-scripts build "src/**/*.ts" && tsc

::endgroup::
::group::@astrojs/vercel:build
cache miss, executing 6312113a19e77d3c

> @astrojs/vercel@5.2.0 build /home/runner/work/astro/astro/packages/integrations/vercel
> astro-scripts build "src/**/*.ts" && tsc

::endgroup::
::group::@astrojs/node:build
cache miss, executing c9229e465c741b9d

> @astrojs/node@6.0.3 build /home/runner/work/astro/astro/packages/integrations/node
> astro-scripts build "src/**/*.ts" && tsc

::endgroup::
::group::@astrojs/mdx:build
cache miss, executing 3f4b9f76d4c9a821

> @astrojs/mdx@1.1.4 build /home/runner/work/astro/astro/packages/integrations/mdx
> astro-scripts build "src/**/*.ts" && tsc

::endgroup::
::group::@astrojs/markdoc:build
cache miss, executing f0419a89d4aa5902

> @astrojs/markdoc@0.7.1 build /home/runner/work/astro/astro/packages/integrations/markdoc
> astro-scripts build "src/**/*.ts" && tsc

::endgroup::
::group::@astrojs/vue:build
cache miss, executing 1ff9cc37f64095ef

> @astrojs/vue@3.0.4 build /home/runner/work/astro/astro/packages/integrations/vue
> astro-scripts build "src/index.ts" && astro-scripts build "src/editor.cts" --force-cjs --no-clean-dist && tsc

::endgroup::
::group::@astrojs/lit:build
cache miss, executing 121f3401436fc307

> @astrojs/lit@3.0.3 build /home/runner/work/astro/astro/packages/integrations/lit
> astro-scripts build "src/**/*.ts" && tsc

::endgroup::
::group::@astrojs/alpinejs:build
cache miss, executing b9c2a02db28dea3c

> @astrojs/alpinejs@0.3.1 build /home/runner/work/astro/astro/packages/integrations/alpinejs
> astro-scripts build "src/**/*.ts" && tsc

::endgroup::
::group::@astrojs/tailwind:build
cache miss, executing a3b65e0aa45b2f69

> @astrojs/tailwind@5.0.2 build /home/runner/work/astro/astro/packages/integrations/tailwind
> astro-scripts build "src/**/*.ts" && tsc

::endgroup::
::group::@astrojs/prefetch:build
cache miss, executing c7ef978a3807c69c

> @astrojs/prefetch@0.4.1 build /home/runner/work/astro/astro/packages/integrations/prefetch
> astro-scripts build "src/**/*.ts" && tsc

::endgroup::
::group::@astrojs/react:build
cache miss, executing d1e1ba0adaaf5417

> @astrojs/react@3.0.4 build /home/runner/work/astro/astro/packages/integrations/react
> astro-scripts build "src/**/*.ts" && tsc

::endgroup::
::group::@astrojs/solid-js:build
cache miss, executing 36055520264d0cf8

> @astrojs/solid-js@3.0.2 build /home/runner/work/astro/astro/packages/integrations/solid
> astro-scripts build "src/**/*.ts" && tsc

::endgroup::
::group::@astrojs/svelte:build
cache miss, executing ee2462332f5dafb7

> @astrojs/svelte@4.0.3 build /home/runner/work/astro/astro/packages/integrations/svelte
> astro-scripts build "src/index.ts" && astro-scripts build "src/editor.cts" --force-cjs --no-clean-dist && tsc

::endgroup::
::group::@astrojs/sitemap:build
cache miss, executing 219a95b8adf9285d

> @astrojs/sitemap@3.0.3 build /home/runner/work/astro/astro/packages/integrations/sitemap
> astro-scripts build "src/**/*.ts" && tsc

::endgroup::

 Tasks:    24 successful, 24 total
Cached:    1 cached, 24 total
  Time:    1m21.876s 

@ematipico
Copy link
Member

@lilnasy I opened a new PR #9057

@lilnasy lilnasy closed this Nov 10, 2023
@lilnasy
Copy link
Contributor Author

lilnasy commented Nov 10, 2023

and not attempt to import the one from the user

@ematipico In this PR, the one from the user does not get imported, notice the .filter(Boolean). Were you under the impression that it did?

@lilnasy lilnasy deleted the fix-integration-defined-middleware branch November 12, 2023 01:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: astro Related to the core `astro` package (scope) pr: docs A PR that includes documentation for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants