[RFC] Only alter Identifiers to avoid conflict. #24
Conversation
| body.append( '\n\n' + exportStatement ); | ||
| } | ||
|
|
||
| intro = introTemplate({ | ||
| amdDeps: bundle.externalModules.length ? '[' + bundle.externalModules.map( quoteId ).join( ', ' ) + '], ' : '', | ||
| names: bundle.externalModules.map( m => bundle.uniqueNames[ m.id ] ).join( ', ' ) | ||
| names: bundle.externalModules.map( m => bundle.uniqueNames[ m.id ] + '__default' ).join( ', ' ) |
There was a problem hiding this comment.
As far as I can tell, this was an existing bug
This is just a request for consideration, so treat it as such. All tests pass, but that doesn't mean I haven't missed something important. The concept here is to do as little modification as possible to the original source, only doing so to avoid conflicts within a scope. This also fixes an issue where all of the top-level scope is accessible, regardless of being exported. It patches this issue by modifying the names of non-exported top-level Identifiers within a module. See added test which fails before this diff.
|
Very much in favour of this - the I was part way through a spot of refactoring (for #17) when you opened this, and got carried away to the extent that |
|
Yes, either way I don't intend for this RFC to be merged as is. Once agreed upon, it should be split into small pieces one thesis each. Doing so will probably yield a better implementation! I'm more than happy to hand you the reigns if you're in the midst of refactoring, or if you would like me to built atop your refactor then I'm happy to do so just say the word. |
|
The first step is probably solving the issues found with more direct fixes, and pulling the tests I added or something like them. |
|
There's definitely a bit more refactoring to do, I found some pretty weird stuff the other day. Deadlines will do that to a library. So it's probably easier if I pay down some of that debt and fix #25, otherwise we'll probably trip over each other. |
|
I'll standby. Lemme know if you have thoughts questions or need help! This Lee Byron On Sun, Dec 28, 2014 at 9:27 PM, Rich Harris notifications@github.com
|
|
Thanks! Really appreciate you raising these issues the past few days, the next version will be a good deal more robust (read: works for more than a handful of my own projects) as a result. |
|
I'd be interested in helping with this to the extent it makes sense. |
|
@eventualbuddha Awesome! For the most part, 0.5.x is already much more relaxed about renaming identifiers, while still avoiding clashes. External modules are still a bit messy, and there are some edge cases where e.g. I think my main priority at the moment, once the last few outstanding issues are closed, is to make the internals a bit less bewildering and to avoid unnecessary work (e.g. checking that identifiers aren't being replaced with the same string). @leebyron btw I had a crack at using Esperanto in place of Smash with Immutable.js, but ran into cycles hell and put it to one side. From the comment breadcrumbs I gather you've been there too... |
|
Yes, I have some interesting circular dependencies that can be solved by careful placement in a single file (smash), but not by ES6 modules. I plan on refactoring to solve the cycle to ready for using modules directly. |
|
I'm going to close this now that you've covered basically everything in it! The output reads so nice now, great work over the last couple weeks! |
|
Thanks! Grateful for the help and input |
This is just a request for consideration, so treat it as such. All tests pass, but that doesn't mean I haven't missed something important. Lookin' for feedback!
The concept here is to do as little modification as possible to the original source, only doing so to avoid conflicts within a scope.
This also fixes an issue where all of the top-level scope is accessible, regardless of being exported. It patches this issue by modifying the names of non-exported top-level Identifiers within a module. See added test which fails before this diff.