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

🐛 BUG: Synchronous version actually returns Promise #911

Open
XiNiHa opened this issue Dec 16, 2023 · 2 comments
Open

🐛 BUG: Synchronous version actually returns Promise #911

XiNiHa opened this issue Dec 16, 2023 · 2 comments
Labels
- P3: minor bug An edge case that only affects very specific usage (priority)

Comments

@XiNiHa
Copy link

XiNiHa commented Dec 16, 2023

What version of @astrojs/compiler are you using?

2.3.4

What package manager are you using?

Yarn Classic

What operating system are you using?

Linux

Describe the Bug

The synchronous version of @astrojs/compiler doesn't actually run synchronously and just returns a Promise.

$ node
Welcome to Node.js v20.10.0.
Type ".help" for more information.
> const compiler = require("@astrojs/compiler/sync")
undefined
> const result = compiler.transform("hi")
undefined
> result
Promise {
  {
    code: 'import {\n' +
      '  Fragment,\n' +
      '  render as $$render,\n' +
      '  createAstro as $$createAstro,\n' +
      '  createComponent as $$createComponent,\n' +
      '  renderComponent as $$renderComponent,\n' +
      '  renderHead as $$renderHead,\n' +
      '  maybeRenderHead as $$maybeRenderHead,\n' +
      '  unescapeHTML as $$unescapeHTML,\n' +
      '  renderSlot as $$renderSlot,\n' +
      '  mergeSlots as $$mergeSlots,\n' +
      '  addAttribute as $$addAttribute,\n' +
      '  spreadAttributes as $$spreadAttributes,\n' +
      '  defineStyleVars as $$defineStyleVars,\n' +
      '  defineScriptVars as $$defineScriptVars,\n' +
      '  renderTransition as $$renderTransition,\n' +
      '  createTransitionScope as $$createTransitionScope,\n' +
      '  createMetadata as $$createMetadata\n' +
      '} from "astro/runtime/server/index.js";\n' +
      '\n' +
      '\n' +
      'export const $$metadata = $$createMetadata("<stdin>", { modules: [], hydratedComponents: [], clientOnlyComponents: [], hydrationDirectives: new Set([]), hoisted: [] });\n' +
      '\n' +
      'const $$Astro = $$createAstro();\n' +
      'const Astro = $$Astro;\n' +
      'const $$stdin = $$createComponent(async ($$result, $$props, $$slots) => {\n' +
      'const Astro = $$result.createAstro($$Astro, $$props, $$slots);\n' +
      'Astro.self = $$stdin;\n' +
      '\n' +
      'return $$render`hi`;\n' +
      "}, '<stdin>', undefined);\n" +
      'export default $$stdin;\n',
    diagnostics: [],
    map: '',
    scope: '5keef2pk',
    css: [],
    scripts: [],
    hydratedComponents: [],
    clientOnlyComponents: [],
    containsHead: false,
    styleError: [],
    propagation: false
  },
  [Symbol(async_id_symbol)]: 210,
  [Symbol(trigger_async_id_symbol)]: 6
}
> result.code
undefined
> (await result).code
'import {\n' +
  '  Fragment,\n' +
  '  render as $$render,\n' +
  '  createAstro as $$createAstro,\n' +
  '  createComponent as $$createComponent,\n' +
  '  renderComponent as $$renderComponent,\n' +
  '  renderHead as $$renderHead,\n' +
  '  maybeRenderHead as $$maybeRenderHead,\n' +
  '  unescapeHTML as $$unescapeHTML,\n' +
  '  renderSlot as $$renderSlot,\n' +
  '  mergeSlots as $$mergeSlots,\n' +
  '  addAttribute as $$addAttribute,\n' +
  '  spreadAttributes as $$spreadAttributes,\n' +
  '  defineStyleVars as $$defineStyleVars,\n' +
  '  defineScriptVars as $$defineScriptVars,\n' +
  '  renderTransition as $$renderTransition,\n' +
  '  createTransitionScope as $$createTransitionScope,\n' +
  '  createMetadata as $$createMetadata\n' +
  '} from "astro/runtime/server/index.js";\n' +
  '\n' +
  '\n' +
  'export const $$metadata = $$createMetadata("<stdin>", { modules: [], hydratedComponents: [], clientOnlyComponents: [], hydrationDirectives: new Set([]), hoisted: [] });\n' +
  '\n' +
  'const $$Astro = $$createAstro();\n' +
  'const Astro = $$Astro;\n' +
  'const $$stdin = $$createComponent(async ($$result, $$props, $$slots) => {\n' +
  'const Astro = $$result.createAstro($$Astro, $$props, $$slots);\n' +
  'Astro.self = $$stdin;\n' +
  '\n' +
  'return $$render`hi`;\n' +
  "}, '<stdin>', undefined);\n" +
  'export default $$stdin;\n'

Link to Minimal Reproducible Example

https://stackblitz.com/edit/stackblitz-starters-skqgj1?file=index.js

@MoustaphaDev
Copy link
Member

MoustaphaDev commented Dec 18, 2023

I think this is because, the internal Transform function returns a promise (the others don't)

return promiseConstructor.New(promiseHandle)

Not sure how it could be refactored though. Maybe @natemoo-re could have more context on this?

@MoustaphaDev MoustaphaDev added the - P3: minor bug An edge case that only affects very specific usage (priority) label Dec 18, 2023
@github-actions github-actions bot removed the needs triage Issue needs to be triaged label Dec 18, 2023
@natemoo-re
Copy link
Member

Oh this was definitely just an oversight. We don't actually use the sync transform function anywhere in our own code.

I think we'd have to remove the ability to pass an async callback for style compilation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P3: minor bug An edge case that only affects very specific usage (priority)
Projects
None yet
Development

No branches or pull requests

3 participants