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: @log does not work on cases prefixed with module name #26

Open
farukg opened this issue Nov 28, 2020 · 1 comment
Open

Bug: @log does not work on cases prefixed with module name #26

farukg opened this issue Nov 28, 2020 · 1 comment

Comments

@farukg
Copy link

farukg commented Nov 28, 2020

All case which have a prefix won't be logged. In the generated js files the code for logging is missing. It took a while to find out but now i have a minimal example:

// RSLoggerBugExample.res
module Page = {
  type t =
    | Home
    | Profile
    | Welcome
}

@react.component
let make = (~page) =>
  @log
  switch page {
  | Page.Welcome => "Hello World"->React.string
  | Home => "Hello Home"->React.string
  | Page.Profile => "Hello Profile"->React.string
  }
// RSLoggerBugExample.bs.js
function RSLoggerBugExample(Props) {
  var page = Props.page;
  switch (page) {
    case /* Home */0 :
        Browser$ReScriptLogger.debug({
              rootModule: "RSLoggerBugExample",
              subModulePath: /* [] */0,
              value: "make",
              fullPath: "RSLoggerBugExample.make",
              filePath: "absolute path to the res file which a can not disable, but that is another issue :D"
            }, "Home");
        return "Hello Home";
    case /* Profile */1 :
        return "Hello Profile";
    case /* Welcome */2 :
        return "Hello World";
  }
}

I got this issue originally in the old reason-syntax. But I could reproduce this bug also in rescript. I'm using [email protected] & [email protected]

@alex35mil
Copy link
Member

It's not a bug per se b/c there are a lot of things can't be displayed in the current implementation of @log attribute just b/c specific construction it's not yet handled in the matcher. I agree it should be handled though.


Implementation note: Ldot should be handled here.

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

No branches or pull requests

2 participants