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: refusing to accept isSelfAccepting for JSX #3521

Merged
merged 8 commits into from Jun 6, 2022

Conversation

bholmesdev
Copy link
Contributor

@bholmesdev bholmesdev commented Jun 3, 2022

Changes

Testing

N/A

Docs

N/A

@changeset-bot
Copy link

changeset-bot bot commented Jun 3, 2022

🦋 Changeset detected

Latest commit: ffde61a

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

This PR includes changesets to release 1 package
Name Type
astro Patch

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 Jun 3, 2022
@bholmesdev bholmesdev changed the title Fix: isSelfAccepting for JSX components in development Fix: refusing to accept isSelfAccepting for JSX Jun 3, 2022
@bholmesdev bholmesdev marked this pull request as draft June 4, 2022 18:52
@bholmesdev bholmesdev force-pushed the fix/is-self-accepting-pt-3-acceptance branch from 2c824ed to 64a311a Compare June 4, 2022 19:22
@bholmesdev bholmesdev marked this pull request as ready for review June 4, 2022 19:54
@bholmesdev bholmesdev requested a review from matthewp June 4, 2022 20:11
Copy link
Contributor

@tony-sull tony-sull left a comment

Choose a reason for hiding this comment

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

This fix looks good to me, just had a question about the hmr.js case would work outside our monorepo

packages/astro/src/core/dev/index.ts Outdated Show resolved Hide resolved
const clientRuntimeFilePaths = clientRuntimeScripts
.map(script => `astro/client/${path.basename(script)}`)
// fixes duplicate dependency issue in monorepo when using astro: "workspace:*"
.filter(filePath => filePath !== 'astro/client/hmr.js');
Copy link
Contributor

Choose a reason for hiding this comment

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

Would this be a problem for astro files not in our monorepo, or is hmr.js always ready before the client needs it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, it's always ready since we inject hmr.js as a hoisted script!

@bholmesdev bholmesdev merged commit 85b9054 into main Jun 6, 2022
@bholmesdev bholmesdev deleted the fix/is-self-accepting-pt-3-acceptance branch June 6, 2022 13:27
@github-actions github-actions bot mentioned this pull request Jun 6, 2022
@okikio
Copy link
Member

okikio commented Jun 7, 2022

I'm still seeing this bug after updating to v1.0.0-beta.41
image

@bholmesdev What do you think?

@chrisblossom
Copy link

This is still happening for me as of v1.0.0-beta.42 if the integration turbolinks() is enabled in astro.config.mjs. @okikio is your issue as well?

@okikio
Copy link
Member

okikio commented Jun 7, 2022

@chrisblossom No, my astro.config.mjs looks like this,

https://github.com/okikio/bundlejs/blob/f7392c07b621023f218319056e5c7e56fa89a91c/astro.config.mjs#L30

import { defineConfig } from "astro/config";

import * as path from "node:path";
import { lookupCollection } from '@iconify/json';

import Icons from 'unplugin-icons/vite';
import IconsResolver from 'unplugin-icons/resolver';
import AutoImport from 'unplugin-auto-import/vite';

import PluginMonacoEditor from "vite-plugin-monaco-editor";
const MonacoEditorPlugin = PluginMonacoEditor.default;

import solid from "@astrojs/solid-js";
import tailwind from "@astrojs/tailwind";
import sitemap from "@astrojs/sitemap";

import { h, s } from "hastscript";

const { icons: fluentIcons } = await lookupCollection("fluent");
const IconLink = fluentIcons["link-24-regular"];
const IconArrow = fluentIcons["arrow-up-right-24-regular"];

const __dirname = path.resolve(path.dirname(""));

// https://astro.build/config
export default defineConfig({
  build: {
    format: "file"
  },
  site: "https://bundlejs.com",
  markdown: {
    rehypePlugins: [
      "rehype-slug",
      [
        "rehype-autolink-headings",
        {
          behavior: "append",
          content: [
            s("svg", {
              xmlns: "http://www.w3.org/2000/svg",
              width: IconLink.width,
              height: IconLink.height,
              viewBox: `0 0 ${IconLink.width} ${IconLink.height}`,
              class: "icon"
            }, [IconLink.body])
          ],
          test: ['h2', 'h3', 'h4', 'h5', 'h6', 'details', 'summary', 'astro-root']
        },
      ],
      ["rehype-external-links", {
        target: "_blank",
        rel: ["noopener"],
        content: [
          s("svg", {
            xmlns: "http://www.w3.org/2000/svg",
            width: IconArrow.width,
            height: IconArrow.height,
            viewBox: `0 0 ${IconArrow.width} ${IconArrow.height}`,
            class: "icon",
            "rehype-icon": "arrow-up-right-24-regular",
          }, [IconArrow.body])
        ]
      }]
    ],
    shikiConfig: {
      theme: "github-dark",
      langs: [],
      wrap: false
    }
  },
  integrations: [
    solid(),
    tailwind({
      config: {
        applyBaseStyles: false
      }
    }),
    sitemap(),
  ],
  vite: {
    build: {
      assetsInlineLimit: 0,
    },
    worker: {
      format: "iife"
    },
    ssr: {
      external: ["svgo"]
    },
    plugins: [
      AutoImport({
        resolvers: [
          IconsResolver({
            prefix: 'Icon',
            extension: 'tsx'
          })
        ]
      }),
      Icons({
        autoInstall: true,
        compiler: 'solid',
        defaultClass: "icon"
      }),
      MonacoEditorPlugin({
        languageWorkers: [],
        customWorkers: [
          { label: "typescript", entry: path.join(__dirname, "./src/scripts/workers/typescript") },
          { label: "json", entry: path.join(__dirname, "./src/scripts/workers/json") },
          { label: "editor", entry: path.join(__dirname, "./src/scripts/workers/editor") },
        ]
      })
    ]
  }
});

SiriousHunter pushed a commit to SiriousHunter/astro that referenced this pull request Feb 3, 2023
* fix: generate client directive scripts from metadata

* chore: changeset

* feat: add all runtime client scripts to optimized deps

* fix: remove hmr.js from optimized deps (monorepo-specific issue)

* Revert "fix: generate client directive scripts from metadata"

This reverts commit 56530a8.

* refactor: move optimizedeps to dev only

* docs: add comment on why optimizdeps

* nit: indentation
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)
Projects
None yet
5 participants