-
Notifications
You must be signed in to change notification settings - Fork 63
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
Consider removing/renaming ngDevMode const declaration from the polyfill #174
Comments
Why not This is a rising standard, iirc |
Sounds reasonable. Created a PR for that: #175 |
Is this declaration preventing anyone from debugging their code with signals? If not, I'd prefer to leave things as they are now, to facilitate later re-upstreaming into Angular. Maybe we can work out a better solution with them. I'm not so sure about that query--sounds like it'll always be false in browsers! |
No, it's not preventing from debugging, it's just that:
At the end of the day, though, if you're planning to postpone any updates to the polyfill code or its documentation until some time later for whatever reason, it's totally fine. |
Can we get angular to support import.meta.env.DEV? |
Yeah let's discuss this with Angular upstream to see what we can do next. |
Currenty, the polyfill depends on the existence of the
ngDevMode
variable in several places to decide whether it should throw useful error messages. This is obviously a copy-paste artifact from the Angular signals codebase.Looks like the variable should be renamed to something framework-agnostic, like
signal-polyfill-debug
, and its usage should be documented in the README. Otherwise, any mentions ofngDevMode
could be removed altogether, and the debug messages could just always be printed.I'd be willing to make a PR for that if needed.
The text was updated successfully, but these errors were encountered: