Skip to content

Commit

Permalink
feat(svelte): warn on non ".css" <link href> (#766)
Browse files Browse the repository at this point in the history
  • Loading branch information
@tivac
tivac authored Jun 21, 2020
1 parent 81e8a0d commit 1ac0e4d
Show file tree
Hide file tree
Showing 12 changed files with 186 additions and 169 deletions.
14 changes: 5 additions & 9 deletions packages/processor/test/at-composes.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,15 +120,11 @@ describe("/processor.js", () => {
});

it("should only allow a single @composes per file", async () => {
try {
await processor.string(id, dedent(`
@composes "./simple.css";
@composes "./blue.css";
`));
} catch({ message }) {
expect(message).toMatch(`Only one @composes rule per file`);
}
await expect(processor.string(id, dedent(`
@composes "./simple.css";
@composes "./blue.css";
`))).rejects.toThrow(`Only one @composes rule per file`);
});
});
});
70 changes: 24 additions & 46 deletions packages/processor/test/composition.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,38 +16,26 @@ describe("/processor.js", () => {
});

it("should fail on invalid composes syntax", async () => {
try {
await processor.string(
"./invalid/value.css",
".a { composes: b from nowhere.css; }"
);
} catch({ message }) {
expect(message).toMatch(`SyntaxError: Expected global or source but "n" found.`);
}
await expect(processor.string(
"./invalid/value.css",
".a { composes: b from nowhere.css; }"
)).rejects.toThrow(`SyntaxError: Expected global or source but "n" found.`);
});

it("should fail if a composition references a non-existant class", async () => {
try {
await processor.string(
"./invalid-composition.css",
".a { composes: b; }"
);
} catch({ message }) {
expect(message).toMatch(`Invalid composes reference`);
}
await expect(processor.string(
"./invalid-composition.css",
".a { composes: b; }"
)).rejects.toThrow(`Invalid composes reference`);
});

it("should fail if a composition references a non-existant file", async () => {
try {
await processor.string(
"./invalid-composition.css",
`.a { composes: b from "../local.css"; }`
);
} catch({ message }) {
expect(message).toMatch(
`Unable to locate "../local.css" from "${path.resolve("invalid-composition.css")}"`
);
}
await expect(processor.string(
"./invalid-composition.css",
`.a { composes: b from "../local.css"; }`
)).rejects.toThrow(
`Unable to locate "../local.css" from "${path.resolve("invalid-composition.css")}"`
);
});

it("should fail if a composition references a non-existant file from a custom resolver", async () => {
Expand All @@ -58,29 +46,19 @@ describe("/processor.js", () => {
],
});

try {
await processor.file(require.resolve("./specimens/composes.css"));
} catch(e) {
const { message } = e;

expect(message).toMatch(
`no such file or directory, open '${require.resolve("./specimens/folder/folder2.css")}a'`
);
}
await expect(processor.file(require.resolve("./specimens/composes.css"))).rejects.toThrow(
`no such file or directory, open '${require.resolve("./specimens/folder/folder2.css")}a'`
);
});

it("should fail on rules that use multiple selectors", async () => {
try {
await processor.string(
"./invalid/composes-first.css",
dedent(`
.a { color: red; }
.b .c { composes: a; }
`)
);
} catch({ message }) {
expect(message).toMatch(`Only simple singular selectors may use composition`);
}
await expect(processor.string(
"./invalid/composes-first.css",
dedent(`
.a { color: red; }
.b .c { composes: a; }
`)
)).rejects.toThrow(`Only simple singular selectors may use composition`);
});

it("should compose a single class", async () => {
Expand Down
24 changes: 9 additions & 15 deletions packages/processor/test/externals.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,24 +15,18 @@ describe("/processor.js", () => {
});

it("should fail if not a valid composition reference", async () => {
try {
await processor.string(
"./invalid-external.css",
dedent(`
:external(some garbage here) { }
`)
);
} catch({ message }) {
expect(message).toMatch(`SyntaxError: Expected`);
}
await expect(processor.string(
"./invalid-external.css",
dedent(`
:external(some garbage here) { }
`)
)).rejects.toThrow(`SyntaxError: Expected`);
});

it("should fail on bad class references", async () => {
try {
await processor.file(require.resolve("./specimens/externals-invalid.css"));
} catch({ message }) {
expect(message).toMatch(`Invalid external reference: nopenopenope`);
}
await expect(processor.file(require.resolve("./specimens/externals-invalid.css"))).rejects.toThrow(
`Invalid external reference: nopenopenope`
);
});

it("should support overriding external values", async () => {
Expand Down
48 changes: 18 additions & 30 deletions packages/processor/test/scoping.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,42 +54,30 @@ describe("/processor.js", () => {
});

it("should not allow :global classes to overlap with local ones (local before global)", async () => {
try {
await processor.string(
"./invalid/global.css",
dedent(`
.a {}
:global(.a) {}
`)
);
} catch({ message }) {
expect(message).toMatch(`Unable to re-use the same selector for global & local`);
}
await expect(processor.string(
"./invalid/global.css",
dedent(`
.a {}
:global(.a) {}
`)
)).rejects.toThrow(`Unable to re-use the same selector for global & local`);
});

it("should not allow :global classes to overlap with local ones (global before local)", async () => {
try {
await processor.string(
"./invalid/global.css",
dedent(`
:global(.a) {}
.a {}
`)
);
} catch({ message }) {
expect(message).toMatch(`Unable to re-use the same selector for global & local`);
}
await expect(processor.string(
"./invalid/global.css",
dedent(`
:global(.a) {}
.a {}
`)
)).rejects.toThrow(`Unable to re-use the same selector for global & local`);
});

it("should not allow empty :global() selectors", async () => {
try {
await processor.string(
"./invalid/global.css",
".a :global() { }"
);
} catch({ message }) {
expect(message).toMatch(`:global(...) must not be empty`);
}
await expect(processor.string(
"./invalid/global.css",
".a :global() { }"
)).rejects.toThrow(`:global(...) must not be empty`);
});
});
});
28 changes: 10 additions & 18 deletions packages/processor/test/values.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,27 +16,19 @@ describe("/processor.js", () => {
});

it("should fail on invalid value syntax", async () => {
try {
await processor.string(
"./invalid/value.css",
"@value foo, bar from nowhere.css"
);
} catch({ message }) {
expect(message).toMatch(`SyntaxError: Expected source but "n" found.`);
}
await expect(processor.string(
"./invalid/value.css",
"@value foo, bar from nowhere.css"
)).rejects.toThrow(`SyntaxError: Expected source but "n" found.`);
});

it("should fail if a value imports a non-existant reference", async () => {
try {
await processor.string(
"./invalid/value.css",
"@value not-real from \"../local.css\";"
);
} catch({ message }) {
expect(message).toMatch(
`Unable to locate "../local.css" from "${path.resolve("invalid/value.css")}"`
);
}
await expect(processor.string(
"./invalid/value.css",
"@value not-real from \"../local.css\";"
)).rejects.toThrow(
`Unable to locate "../local.css" from "${path.resolve("invalid/value.css")}"`
);
});

it("shouldn't replace values unless they're safe", async () => {
Expand Down
7 changes: 6 additions & 1 deletion packages/rollup/test/rollup.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ describe("/rollup.js", () => {
expect(exists("./output/no-css/assets/no-css.css")).toBe(false);
});

// eslint-disable-next-line jest/expect-expect
it("should ignore external modules", async () => {
const bundle = await rollup({
input : require.resolve("./specimens/external.js"),
Expand Down Expand Up @@ -307,6 +308,7 @@ describe("/rollup.js", () => {
expect(result).toMatchRollupCodeSnapshot();
});

// eslint-disable-next-line jest/expect-expect
it("should warn that styleExport and done aren't compatible", async () => {
const { logSnapshot } = logs("warn");

Expand Down Expand Up @@ -539,6 +541,7 @@ describe("/rollup.js", () => {
expect(result).toMatchRollupCodeSnapshot();
});

// eslint-disable-next-line jest/expect-expect
it("should log in verbose mode", async () => {
const { logSnapshot } = logs();

Expand Down Expand Up @@ -711,7 +714,7 @@ describe("/rollup.js", () => {
.catch((e) => expect(e.toString()).toMatch(".wooga"))
);


// eslint-disable-next-line jest/expect-expect
it("should throw errors in in before plugins", () =>
rollup({
input : require.resolve("./specimens/simple.js"),
Expand All @@ -726,6 +729,7 @@ describe("/rollup.js", () => {
.catch(checkError)
);

// eslint-disable-next-line jest/expect-expect
it("should throw errors in after plugins", () =>
rollup({
input : require.resolve("./specimens/simple.js"),
Expand All @@ -741,6 +745,7 @@ describe("/rollup.js", () => {
);

// Skipped because I can't figure out how to catch the error being thrown?
// eslint-disable-next-line jest/no-disabled-tests
it.skip("should throw errors in done plugins", () =>
rollup({
input : require.resolve("./specimens/simple.js"),
Expand Down
4 changes: 4 additions & 0 deletions packages/svelte/svelte.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,10 @@ module.exports = (config = false) => {
return out;
}

if(!href.endsWith(".css")) {
console.warn(`Possible invalid <link> href: ${href}`);
}

out.push({
link,
href,
Expand Down
8 changes: 8 additions & 0 deletions ...te/test/__snapshots__/rollup.test.js.snap → .../__snapshots__/svelte-rollup.test.js.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,14 @@ Object {
}
`;
exports[`/svelte.js rollup errors should show useful errors from rollup (non-css file) 1`] = `
Array [
Array [
"Possible invalid <link> href: ./error-link.html",
],
]
`;
exports[`/svelte.js rollup watching should generate updated output 1`] = `
Snapshot Diff:
- First value
Expand Down
8 changes: 8 additions & 0 deletions packages/svelte/test/__snapshots__/svelte.test.js.snap
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`/svelte.js should expose CSS errors in a useful way (non-css file) 1`] = `
Array [
Array [
"Possible invalid <link> href: ./error-link.html",
],
]
`;
exports[`/svelte.js should extract CSS from a <link> tag (existing script) 1`] = `
"<div class=\\"flex wrapper\\">
<h1 class=\\"flex fooga hd\\">Head</h1>
Expand Down
1 change: 1 addition & 0 deletions packages/svelte/test/specimens/error-link-non-css.html
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<link rel="stylesheet" href="./error-link.html" />
Loading

0 comments on commit 1ac0e4d

Please sign in to comment.