AS3ExternsGenerator: Generate externs under the flash package name too#11
AS3ExternsGenerator: Generate externs under the flash package name too#11Fancy2209 wants to merge 15 commits intoopenfl:masterfrom
flash package name too#11Conversation
flash package nameflash package name too
scripts/AS3ExternsGenerator.hx
Outdated
| shouldWriteAgain = true; | ||
| outputFilePath = outputFilePath.replace("/lib/" + originalName + "/", "/lib/" + newName + "/"); | ||
| } | ||
| if (generated.contains(originalName)) { |
There was a problem hiding this comment.
Doing a replace over the entire file contents using the package name only feels dangerous. It could lead to things getting replaced that shouldn't be replaced. If someone wanted to use this option to remap "com", that would not work as well as "openfl", which is probably safer as long as things are case-sensitive. I've been trying to write the generators in a way that they might be useful to other projects too.
A safer version might be to replace "package " + originalName and ":" + originalName. Or maybe regular expressions, to handle potential whitespace variations and things.
// for AS3
new EReg("package\\s+" + originalName + "(?=\\.|\\s*{)", "g");
new EReg(":\\s+" + originalName + "(?=\\.)", "g");However, even with this approach, doc comments may still include some replacements. There may not be a way to avoid that without calling the generator twice and remapping at an earlier stage of the process.
Not as important, but just something I that came to mind. I assume that this is inspired by Haxe's --remap option. I wonder if --remap supports sub-packages, like --remap com.example:org.example. It would be good to either support that, or to throw an exception if a . character is contained in either the original or new name.
There was a problem hiding this comment.
I think using regular expressions may be the solution here, I feel like if an good approach for manipulating the end generated file could be cleaner
I am not sure about the subpackages.
Should I just try using the eregs?
There was a problem hiding this comment.
Should I just try using the eregs?
Yeah, go for it.
I realized that you might need one for imports too.
new EReg("import\\s+" + originalName + "(?=\\.)", "g");There was a problem hiding this comment.
Finally committed!
TSExternGenerator: Remove package remapping as it is useless for TS
|
All the AS3ExternGenerator changes were superceeded by 42f0846 |
cf4580e to
75e6eb3
Compare
|
Should I, for the flash package, rename Error, IllegalOperationError, IOError, ArgumentError, TypeError, SecurityError, RangeError to the top-level? |
|
There’s some overlap with top level error types in AS3 and JS. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error#error_types I guess it made make sense to expose the ones from AS3 that don’t also exist in JS. Hopefully, the ones that are shared are mostly compatible in terms of APIs. |
errorID only exists in the OpenFL/AS3 Versions, but yeah I'll rename only the non overlapping ones then |
|
it seems the last commit didn't work, do I need to add the "" package to the package list for it to work? |
If it were to work for these error classes, it would probably add every class in every package as well. Maybe we need a new option to include specific symbols too, to allow adding classes outside of the included packages. |
|
I added includedSymbols but it seems symbol renaming doesn't handle needing additional imports due to a Class no longer beeing part of a specific package |
fef11e8 to
00dcb51
Compare
00dcb51 to
ed815fe
Compare
|
It works now |
Poluting top-level by default isn't very good, so we should probably have a third swc for that that'll have things like that
|
I've been thinking and it may make more sense for package remapping to be added to Royale. Flash package will cause issues when you try to use Starling, as to the compiler, openfl.* and flash.* are different. |
lib/openfl/index.js was modified to export openfl under window.flash
TS was excluded since, from my understanding, webpack alias allow for the same thing