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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃挕 RFC: Better error reporting for failed parsing/printing #168

Closed
1 of 3 tasks
jasikpark opened this issue Apr 5, 2022 · 1 comment 路 Fixed by #246
Closed
1 of 3 tasks

馃挕 RFC: Better error reporting for failed parsing/printing #168

jasikpark opened this issue Apr 5, 2022 · 1 comment 路 Fixed by #246

Comments

@jasikpark
Copy link
Collaborator

Background & Motivation

I want to get a clearer output from this plugin when it is fed:

---
const {label, name, class, ...htmlProps} = Astro.props;
---

<style lang="scss">
  .label-text {
    font-size: 14px;
    margin-bottom: 8px;
  };

  input {
    display: block;
    box-sizing: border-box;
    height: 48px;
    border: none;
    outline: none;
    border-radius: 4px;
    box-shadow: inset 0 0 0 1px var(--theme-input-border);
    background-color: transparent;
    padding: 16px;
    font-weight: 600;
    font-size: 16px;
    color: var(--color-input-text);
    margin-bottom: 24px;

    &:focus {
      /* Visible in Windows high-contrast themes */
      outline-color: transparent;
      outline-width: 2px;
      outline-style: dotted;
      box-shadow: inset 0 0 0 2px var(--theme-accent);
    }
    &::placeholder {
      color: var(--theme-text-disabled);
      font-weight: 500;
    }

    &:disabled {
      background-color: var(--theme-button-dim);
      color: var(--theme-text-disabled);

      &::placeholder {
        color: var(--theme-text-disabled);
      }
    }

    /*
    * HACK: color-scheme dark wreaks havoc, so we have to try to override the styles
    */
    &:-webkit-autofill {
      box-shadow: 0 0 0 100px var(--theme-bg) inset;
      border: 2px solid var(--theme-input-border);
      padding: 14px; /* Since we are changing from inset box-shadow to border, change padding to prevent layout shifts */
      -webkit-text-fill-color: var(--theme-text) !important;

      &:focus {
        box-shadow: 0 0 0 100px var(--theme-bg) inset;
        border: 2px solid var(--theme-accent);
        padding: 14px;
      }
    }
  }

  
</style>

<label>
  <div class='label-text'>{label}</div>
  <input class="width-100" name={name} {...htmlProps}/>
</label>

in v0.1.0-next.4 I get the output:

[error] src/components/atoms/Input.astro: Error: Unhandled node type "frontmatter"!
[error]     at Object.print (/Users/calebjasik-defined/Github/defined.net/www/node_modules/prettier-plugin-astro/dist/index.js:448:19)
[error]     at callPluginPrintFunction (/Users/calebjasik-defined/Github/defined.net/www/node_modules/prettier/index.js:8447:26)
[error]     at mainPrintInternal (/Users/calebjasik-defined/Github/defined.net/www/node_modules/prettier/index.js:8396:22)
[error]     at mainPrint (/Users/calebjasik-defined/Github/defined.net/www/node_modules/prettier/index.js:8383:18)
[error]     at /Users/calebjasik-defined/Github/defined.net/www/node_modules/prettier/index.js:8238:27
[error]     at AstPath.each (/Users/calebjasik-defined/Github/defined.net/www/node_modules/prettier/index.js:8230:11)
[error]     at AstPath.map (/Users/calebjasik-defined/Github/defined.net/www/node_modules/prettier/index.js:8237:14)
[error]     at Object.print (/Users/calebjasik-defined/Github/defined.net/www/node_modules/prettier-plugin-astro/dist/index.js:275:48)
[error]     at callPluginPrintFunction (/Users/calebjasik-defined/Github/defined.net/www/node_modules/prettier/index.js:8447:26)
[error]     at mainPrintInternal (/Users/calebjasik-defined/Github/defined.net/www/node_modules/prettier/index.js:8396:22)

similar to 0.1.0-next.0 in #165.

The bug here discovered by @antonyfaris is that I'm trying to bind to class which is a reserved name, so parsing fails.

I would prefer there was a clearer error message such as "error: variable cannot be bound to class which is a reserved name" similar to how the TS parser works: prettier playground link from @antonyfaris

Proposed Solution

Possible solutions

Pass through error messages from child parsers of the plugin

Alternatives considered

Risks, downsides, and/or tradeoffs

Open Questions

Detailed Design

No response

Help make it happen!

  • I am willing to submit a PR to implement this change.
  • I am willing to submit a PR to implement this change, but would need some guidance.
  • I am not willing to submit a PR to implement this change.
@tonivj5
Copy link

tonivj5 commented May 20, 2022

Same error happens formating this file (caused by highlighted line): https://github.com/withastro/astro/blob/main/examples/docs/src/layouts/MainLayout.astro#L13

Also, I've found another unexpected behavior with that line:

  • if you comment it (using // or /* */) and run formatting, the formater breaks the regex
  • Repro:
const { content = {} } = Astro.props;
const currentPage = new URL(Astro.request.url).pathname;
// const currentFile = `src/pages${currentPage.replace(/\/$/, '')}.md`;
const githubEditUrl = CONFIG.GITHUB_EDIT_URL && CONFIG.GITHUB_EDIT_URL + currentFile;
  • Run formatting
const { content = {} } = Astro.props;
const currentPage = new URL(Astro.request.url).pathname;
// const currentFile = `src/pages${currentPage.replace(//$/, '')}.md`;
//                                                      ^_ regex has changed! broken, indeed
const githubEditUrl = CONFIG.GITHUB_EDIT_URL && CONFIG.GITHUB_EDIT_URL + currentFile;
  • if you uncomment the line, you'll see the code is broken now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants