Skip to content

Commit

Permalink
Clear todos and remove css kebab handling (#8019)
Browse files Browse the repository at this point in the history
  • Loading branch information
bluwy committed Aug 10, 2023
1 parent a39ff7e commit 34cb200
Show file tree
Hide file tree
Showing 9 changed files with 45 additions and 55 deletions.
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}`;
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', () => {});

0 comments on commit 34cb200

Please sign in to comment.