-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Conversation
🦋 Changeset detectedLatest commit: ffde61a The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
isSelfAccepting
for JSX components in developmentisSelfAccepting
for JSX
2c824ed
to
64a311a
Compare
There was a problem hiding this 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
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'); |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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!
I'm still seeing this bug after updating to @bholmesdev What do you think? |
This is still happening for me as of |
@chrisblossom No, my 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") },
]
})
]
}
}); |
* 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
Changes
optimizedDeps.include
. We excludehmr.js
from this list due to dependency issues in the monorepo when usingworkspace:*
. I'm unable to find exactly what's causing this, but the issue seems urgent enough to roll with this temporary solution.Testing
N/A
Docs
N/A