Skip to content

Commit

Permalink
Implement new default script behavior, style is:global (withastro…
Browse files Browse the repository at this point in the history
…#2961)

* feat: implement RFC0016

* chore: update to latest compiler

* chore: update compiler

* test: fix script tests

* test: update public tests

* feat: throw a warning when referencing scripts in `public/` without `is:inline`
  • Loading branch information
natemoo-re committed Apr 2, 2022
1 parent 42760e0 commit d55658f
Show file tree
Hide file tree
Showing 18 changed files with 46 additions and 32 deletions.
5 changes: 5 additions & 0 deletions .changeset/happy-carrots-carry.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': minor
---

Implement [RFC0016](https://github.com/withastro/rfcs/blob/main/proposals/0016-style-script-defaults.md) which changes the default behavior of `script`, introduces `is:inline`, and changes `<style global>` to `<style is:global>`
2 changes: 1 addition & 1 deletion examples/component/demo/src/pages/index.astro
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import * as Component from '@example/my-component';
<meta charset="utf-8" />
<meta name="viewport" content="width=device-width" />
<title>Welcome to Astro</title>
<style global>
<style is:global>
h {
display: block;
font-size: 2em;
Expand Down
5 changes: 3 additions & 2 deletions examples/with-tailwindcss/src/components/Button.astro
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
<button class="py-2 px-4 bg-purple-500 text-white font-semibold rounded-lg shadow-md hover:bg-purple-700 focus:outline-none focus:ring-2 focus:ring-purple-400 focus:ring-opacity-75">
<slot />
</button>
<script hoist>

<script>
import confetti from 'canvas-confetti';
document.body.querySelector('button').addEventListener("click", () => confetti());
</script>
</script>
2 changes: 1 addition & 1 deletion packages/astro/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@
"test:match": "mocha --timeout 20000 -g"
},
"dependencies": {
"@astrojs/compiler": "^0.13.2",
"@astrojs/compiler": "^0.14.1",
"@astrojs/language-server": "^0.13.2",
"@astrojs/markdown-remark": "^0.7.0",
"@astrojs/prism": "0.4.1",
Expand Down
9 changes: 4 additions & 5 deletions packages/astro/src/core/render/result.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,10 +122,9 @@ export function createResult(args: CreateResultArgs): SSRResult {
let extra = `This can be replaced with a dynamic import like so: await import("${path}")`;
if (isCSSRequest(path)) {
extra = `It looks like you are resolving styles. If you are adding a link tag, replace with this:
<style global>
@import "${path}";
</style>
---
import "${path}";
---
`;
} else if (isScriptRequest(path)) {
extra = `It looks like you are resolving scripts. If you are adding a script tag, replace with this:
Expand All @@ -134,7 +133,7 @@ export function createResult(args: CreateResultArgs): SSRResult {
or consider make it a module like so:
<script type="module" hoist>
<script>
import MyModule from "${path}";
</script>
`;
Expand Down
7 changes: 4 additions & 3 deletions packages/astro/src/runtime/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -535,16 +535,17 @@ function getHTMLElementName(constructor: typeof HTMLElement) {
}

function renderElement(name: string, { props: _props, children = '' }: SSRElement, shouldEscape = true) {
// Do not print `hoist`, `lang`, `global`
// Do not print `hoist`, `lang`, `is:global`
const { lang: _, 'data-astro-id': astroId, 'define:vars': defineVars, ...props } = _props;
if (defineVars) {
if (name === 'style') {
if (props.global) {
if (props['is:global']) {
children = defineStyleVars(`:root`, defineVars) + '\n' + children;
} else {
children = defineStyleVars(`.astro-${astroId}`, defineVars) + '\n' + children;
}
delete props.global;
delete props['is:global'];
delete props['is:scoped'];
}
if (name === 'script') {
delete props.hoist;
Expand Down
12 changes: 10 additions & 2 deletions packages/astro/src/vite-plugin-astro/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ export default function astro({ config, logging }: AstroPluginOptions): vite.Plu
const { query: fromQuery, filename } = parseAstroRequest(from);
if (fromQuery.astro && isRelativePath(id) && fromQuery.type === 'script') {
const resolvedURL = new URL(id, `file:https://${filename}`);
const resolved = resolvedURL.pathname;
const resolved = resolvedURL.pathname
if (isBrowserPath(resolved)) {
return relativeToRoot(resolved + resolvedURL.search);
}
Expand Down Expand Up @@ -96,7 +96,7 @@ export default function astro({ config, logging }: AstroPluginOptions): vite.Plu
let source = await fs.promises.readFile(fileUrl, 'utf-8');
const isPage = fileUrl.pathname.startsWith(config.pages.pathname);
if (isPage && config._ctx.scripts.some((s) => s.stage === 'page')) {
source += `\n<script hoist src="${PAGE_SCRIPT_ID}" />`;
source += `\n<script src="${PAGE_SCRIPT_ID}" />`;
}
if (query.astro) {
if (query.type === 'style') {
Expand Down Expand Up @@ -127,6 +127,14 @@ export default function astro({ config, logging }: AstroPluginOptions): vite.Plu
throw new Error(`No hoisted script at index ${query.index}`);
}

if (hoistedScript.type === 'external') {
const src = hoistedScript.src!;
if (src.startsWith('/') && !isBrowserPath(src)) {
const publicDir = config.public.pathname.replace(/\/$/, '').split('/').pop() + '/';
throw new Error(`\n\n<script src="${src}"> references an asset in the "${publicDir}" directory. Please add the "is:inline" directive to keep this asset from being bundled.\n\nFile: ${filename}`)
}
}

return {
code: hoistedScript.type === 'inline' ? hoistedScript.code! : `import "${hoistedScript.src!}";`,
};
Expand Down
2 changes: 1 addition & 1 deletion packages/astro/test/astro-public.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ describe('Public', () => {
it('css and js files do not get bundled', async () => {
let indexHtml = await fixture.readFile('/index.html');
expect(indexHtml).to.include('<script src="/example.js"></script>');
expect(indexHtml).to.include('<link href="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/example.css" ref="stylesheet">');
expect(indexHtml).to.include('<link href="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/example.css" rel="stylesheet">');
expect(indexHtml).to.include('<img src="/images/twitter.png">');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
}
</style>

<style global>
<style is:global>
html {
--primary: aquamarine;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
<html lang="en">
<head>
<title>This Site</title>
<link href="/example.css" ref="stylesheet"/>
<script src="/example.js"></script>
<link href="/example.css" rel="stylesheet"/>
<script is:inline src="/example.js"></script>
</head>
<body>
<img src="/images/twitter.png" />
</body>
</html>
</html>
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
<script hoist type="module">
<script>
console.log('some content here.');
</script>
</script>
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
---
import url from "../scripts/no_hoist_nonmodule.js?url"
---
<script src={url}></script>
<script is:inline src={url}></script>
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
---
import url from '../scripts/no_hoist_module.js?url';
---
<script type="module" src={url}></script>
<script is:inline type="module" src={url}></script>
Original file line number Diff line number Diff line change
@@ -1 +1 @@
<script hoist type="module" src="../scripts/something.js"></script>
<script src="../scripts/something.js"></script>
Original file line number Diff line number Diff line change
@@ -1 +1 @@
<script hoist type="module" src="../scripts/another_external.js"></script>
<script src="../scripts/another_external.js"></script>
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,4 @@ import Widget2 from '../components/Widget2.astro';
<Widget />
<Widget2 />
</body>
</html>
</html>
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<style global>
<style is:global>
/* Testing remote imports */
@import "https://unpkg.com/open-props";
@import "https://unpkg.com/open-props/normalize.min.css";
</style>
</style>
8 changes: 4 additions & 4 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit d55658f

Please sign in to comment.