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

Clear todos and remove css kebab handling #8019

Merged
merged 1 commit into from
Aug 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions .changeset/giant-plants-sip.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
---
'astro': major
---

Remove backwards-compatible kebab-case transform for camelCase CSS variable names passed to the `style` attribute. If you were relying on the kebab-case transform in your styles, make sure to use the camelCase version to prevent missing styles. For example:

```astro
---
const myValue = "red"
---

<!-- input -->
<div style={{ "--myValue": myValue }}></div>

<!-- output (before) -->
<div style="--my-value:var(--myValue);--myValue:red"></div>

<!-- output (after) -->
<div style="--myValue:red"></div>
```

```diff
<style>
div {
- color: var(--my-value);
+ color: var(--myValue);
}
</style>
```
4 changes: 1 addition & 3 deletions packages/astro/e2e/css-sourcemaps.test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { expect } from '@playwright/test';
import { isWindows, testFactory } from './test-utils.js';
import { testFactory } from './test-utils.js';

const test = testFactory({
root: './fixtures/css/',
Expand All @@ -16,8 +16,6 @@ test.afterAll(async () => {
});

test.describe('CSS Sourcemap HMR', () => {
test.skip(isWindows, 'TODO: fix css hmr in windows');

test('removes Astro-injected CSS once Vite-injected CSS loads', async ({ page, astro }) => {
const html = await astro.fetch('/').then((res) => res.text());

Expand Down
4 changes: 1 addition & 3 deletions packages/astro/e2e/css.test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { expect } from '@playwright/test';
import { getColor, isWindows, testFactory } from './test-utils.js';
import { getColor, testFactory } from './test-utils.js';

const test = testFactory({
root: './fixtures/css/',
Expand All @@ -16,8 +16,6 @@ test.afterAll(async () => {
});

test.describe('CSS HMR', () => {
test.skip(isWindows, 'TODO: fix css hmr in windows');

test('edit CSS from @import', async ({ page, astro }) => {
await page.goto(astro.resolveUrl('/'));

Expand Down
8 changes: 2 additions & 6 deletions packages/astro/e2e/multiple-frameworks.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -154,9 +154,7 @@ test.skip('Multiple frameworks', () => {
await expect(count, 'initial count updated to 5').toHaveText('5');
});

// TODO: re-enable this test when #3559 is fixed
// https://github.com/withastro/astro/issues/3559
test.skip('Vue component', async ({ astro, page }) => {
test('Vue component', async ({ astro, page }) => {
await page.goto(astro.resolveUrl('/'));

const count = page.locator('#vue-counter pre');
Expand All @@ -169,9 +167,7 @@ test.skip('Multiple frameworks', () => {
await expect(count, 'initial count updated to 5').toHaveText('5');
});

// TODO: track down a reliability issue in this test
// It seems to lost connection to the vite server in CI
test.skip('Svelte component', async ({ astro, page }) => {
test('Svelte component', async ({ astro, page }) => {
await page.goto(astro.resolveUrl('/'));

const count = page.locator('#svelte-counter pre');
Expand Down
6 changes: 0 additions & 6 deletions packages/astro/src/cli/add/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -216,12 +216,6 @@ export async function add(names: string[], { flags }: AddOptions) {
await fs.writeFile(fileURLToPath(configURL), ASTRO_CONFIG_STUB, { encoding: 'utf-8' });
}

// TODO: improve error handling for invalid configs
if (configURL?.pathname.endsWith('package.json')) {
throw new Error(
`Unable to use "astro add" with package.json configuration. Try migrating to \`astro.config.mjs\` and try again.`
);
}
let ast: t.File | null = null;
try {
ast = await parseAstroConfig(configURL);
Expand Down
1 change: 0 additions & 1 deletion packages/astro/src/core/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,6 @@ export function emoji(char: string, fallback: string) {
* through a script tag or a dynamic import as-is.
*/
// NOTE: `/@id/` should only be used when the id is fully resolved
// TODO: Export a helper util from Vite
export async function resolveIdToUrl(loader: ModuleLoader, id: string, root?: URL) {
let resultId = await loader.resolveId(id, undefined);
// Try resolve jsx to tsx
Expand Down
20 changes: 2 additions & 18 deletions packages/astro/src/jsx/server.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { AstroError } from '../core/errors/errors.js';
import { AstroJSX, jsx } from '../jsx-runtime/index.js';
import { renderJSX } from '../runtime/server/jsx.js';

Expand All @@ -22,7 +23,7 @@ export async function check(
// if the exception is from an mdx component
// throw an error
if (Component[Symbol.for('mdx-component')]) {
throw createFormattedError({
throw new AstroError({
message: error.message,
title: error.name,
hint: `This issue often occurs when your MDX component encounters runtime errors.`,
Expand Down Expand Up @@ -51,23 +52,6 @@ export async function renderToStaticMarkup(
return { html };
}

type FormatErrorOptions = {
message: string;
name: string;
stack?: string;
hint: string;
title: string;
};
// TODO: Remove this function and use `AstroError` when we refactor it to be usable without error codes
function createFormattedError({ message, name, stack, hint }: FormatErrorOptions) {
const error = new Error(message);
error.name = name;
error.stack = stack;
// @ts-expect-error - hint is not part of the Error interface but it will be picked up by the error overlay
error.hint = hint;
return error;
}

export default {
check,
renderToStaticMarkup,
Expand Down
4 changes: 0 additions & 4 deletions packages/astro/src/runtime/server/render/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,6 @@ const toStyleString = (obj: Record<string, any>) =>
Object.entries(obj)
.map(([k, v]) => {
if (k[0] !== '-' && k[1] !== '-') return `${kebab(k)}:${v}`;
// TODO: Remove in v3! See #6264
// We need to emit --kebab-case AND --camelCase for backwards-compat in v2,
// but we should be able to remove this workaround in v3.
if (kebab(k) !== k) return `${kebab(k)}:var(${k});${k}:${v}`;
Comment on lines -32 to -35
Copy link
Member

Choose a reason for hiding this comment

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

♥️

return `${k}:${v}`;
})
.join(';');
Expand Down
24 changes: 10 additions & 14 deletions packages/astro/test/astro-external-files.test.js
Original file line number Diff line number Diff line change
@@ -1,22 +1,18 @@
/**
* UNCOMMENT: add support for smarter "external" scripts in Rollup
import { expect } from 'chai';
import { loadFixture } from './test-utils.js';

let fixture;
describe('External file references', () => {
let fixture;

before(async () => {
fixture = await loadFixture({ root: './fixtures/astro-external-files/' });
await fixture.build();
});
before(async () => {
fixture = await loadFixture({ root: './fixtures/astro-external-files/' });
await fixture.build();
});

// TODO: Vite error: fix external files
describe('Externeal file references', () => {
it('Build with externeal reference', async () => {
let rss = await fixture.readFile('/index.html');
expect(rss).to.be(''); // TODO: inline snapshot
});
it('Build with externeal reference', async () => {
const html = await fixture.readFile('/index.html');
expect(html).to.include('<script src="/external-file.js"');
});
});
*/

it.skip('is skipped', () => {});
Loading