-
Notifications
You must be signed in to change notification settings - Fork 135
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
Initialization, scopes and assignments - possible bugs / possible improvements #167
Comments
I see there's an existing bug for Assignment to the macro failing to trigger anything seems like a major issue to me. That needs its own ticket and I imagine a fix will be reasonably high priority.
I'd argue that's the expected behavior. Macros are compile-time so they will never undergo initialization. It also means there would be never be a reason not to initialize a macro at the top of a scope, or better at the top of a file. After all, you can't do anything clever like use an if conditional to decide which macro to use. I'd say warning when macros are not hoisted above runtime code (either in a block or in a whole file) would be an excellent thing to include in a lint rule. As far as your suggestion RE scopes, I'd be curious to hear more about your use case. plugin-macros has no desire to stand in for all the use cases that can be supported when writing a full custom babel plugin. I know it's a little fuzzy because in some developers will have the ability to write macros but not plugins. |
I've been experimenting with nested macros, and with stateful macros. // This returns array of files in a directory
// https://www.npmjs.com/package/files.macro
import files from "files.macro";
// Let say that this filters an array of paths by extension
import filterPaths from "./filter-paths.macro";
// Configuration without a config file (stateful macro)
// Easy to do if constant values are passed like here.
filterPaths.config = {
endsWith: ".macro.js",
};
// --- Nesting macros ----
// This works but only when "files.macro" is imported first:
// A
const macrosInDir = filterPaths(files("./js"));
// B
const arr = files("./js");
const macrosInDir2 = filterPaths(arr); It won't work when filterPaths is imported first because Probably for things like this using a preval/preval.macro would be a better choice. |
So this is the package that I've been working on. |
Oh my. I'm not sure how I feel about this. It feels to me like the inner platform effect. You're certainly hitting the limits of |
That he doesn't have to write a macro. It allows a user to use packages that he is familiar with in a new way right of the box. If there is a package that does something (lists files in a directory / validates enviromental variable / synchronously reads a file/html/image/data / and so on), with this wrapper it can be used at build-time without any extra steps. Of couse you can do it with preval.macro too, but with this you can do it with less code and in some alternative ways. Also, because it's all managed by one package, you can safely nest macros when you use only one import. User can also write a custom codegen/preval and many more things in few lines of code even if he don't know anything about babel or AST. That's at least how I see it. I definitly will use it myself for some things, even though I'm not sure how useful it will turnout in the long run. It was mainly a cool project for few nights. I partially agree with that 'inner platform effect', but I don't think it's that far from things like preval either. It just sorts macros before processing them, and allows some alternative syntaxes. |
Writing a macro shouldn't be scary, especially since it's just writing cjs/node code. There's tons of resources, documentation, and assistance in helping users with that. The maintainers of this project are invested in making it as painless as possible. Your way would create a new node-embedded-in-plain-js-with-macros environment for which there would be virtually no support. I guess I don't see why any library needs to be usable as a macro. You can certainly use any library you like in a macro. And fixing babel templating would make it way, way, way easier for people who don't know AST stuff to do work in babel. I'm specifically referring to this issue, but there would be other advantages to the approach I have in mind, e.g. often a particular code fragment could be interpreted as an expression or a statement and it won't be obvious which is needed until it is known where in the code that fragment belongs. A proper templating solution would understand that when a expression was inserted into a location expecting a statement, it should be wrapped in an ExpressionStatement. That alone should nearly eliminate the current need for plugin authors to express their code using marginally documented AST node constructors. |
They did a great job. Don't get me wrong. It's not like I think that there is something wrong with current API. I've just noticed, that a lot of macros listed here are basic wrappers around some functions anyway (files.macro, fast-fp.macro, data-uri.macro, paths.macro, ms.macro and more...). I thought that even though babel-plugin-macros is awsome for macros like this one, it requires some unnecessary boilerplate for basic use cases like the ones that I've listed above. So I fiddled around a bit to see if I could find a solution to that. My package doesn't have to be used to turn modules to macros, it can be used to create functional standalone macros too. It just reduces all AST related boilerplate in that case.
I know that the detailed READMY may suggests something else, but actually I'm not excepting that many people will start to use it. I've been using NPM for a while now, and I wanted to see how long it takes to create a package with a decent README. I'll use it, if some people will find a use for it I'll be happy, and if no one will, that's fine too. After all, your arguments against an approach like this are very fair and I appreciate them. |
Ah, but that still confounds linters and type checkers... |
Don't worry, unless more people will start to use it (which is unlikely) I don't plan on developing this package any further anyway as it already fulfills all my needs.
Open source seems like a good way to learn a lot so I will gladly give it a shot one day, but unfortunately I can't afford to volunteer in such projects right now.
Yes. That's the issue with things like that, but there are probably ways to make it work too. Thanks for your opinions and advice. Good luck with you work, too. |
babel-plugin-macros
version: 3.0.1node
version: v14.15.1npm
version: 6.14.8Relevant code or config
Problem description and suggested solutions:
I can see few issues here:
You can access a macro in a scope before initialization. It's a bit unexpected and there are no warnings/errors. It may cause some issues especially if there are some global variables.
When you do assignments, direct assignment is completely ignored.
Even if the macro was imported or required as const there is no error.
I assume that it has something to do with global variables, but I think
it would be nice, if a user could handle a scenario like this in a macro.
Either to just print an error message like 'usage like this is not allowed' or
to do something else.
Babel-plugin-macros allows for scoping (awesome), but the way it's handled right now
makes it impossible to handle all macro usages within a single function (without some nasty tricks, atleast).
Maybe there is a way to handle scoping better? Sometimes user may want to do some global post-processing in a file after processing all macros. References could be organised like this for example:
I gives more context about the macro usage.
Of course, localName name currently can be easily checked with
Identifier
name prop.The main issue are scopes.
Also two minor things.
require()
) is not recognized. Not that I need it, it's just something I noticed. It looks like a minor oversight, but maybe it's intentional.The text was updated successfully, but these errors were encountered: