Opened 9 years ago
Closed 8 years ago
#14465 closed defect (wontfix)
[patch] [cla] dojox.storage in async mode
Reported by: | Martin Repta | Owned by: | dylan |
---|---|---|---|
Priority: | low | Milestone: | 2.0 |
Component: | Storage/Flash | Version: | 1.7.1 |
Keywords: | dojox, storage | Cc: | Jens Arps |
Blocked By: | Blocking: |
Description
dojox.storage system does not work with AMD. It must to be converted into new syntax.
I did this conversion.
Attachments (1)
Change History (17)
Changed 9 years ago by
Attachment: | dojox_storage_patch.txt added |
---|
comment:1 Changed 9 years ago by
Thanks for the conversion. Re your comment in this ticket description
does not work
is anything actually not working, or that's just a strange wording?
Also, you'll need to file a CLA before we can look at your patch, thanks! Let us know when you send it in.
The other thing is that generally all dependencies should be in the top list, in the define() call, rather than having scattered asynchronous require() calls.
comment:2 Changed 9 years ago by
Component: | Dojox → Storage/Flash |
---|---|
Owner: | changed from Adam Peller to Shane O'Sullivan |
comment:3 Changed 9 years ago by
Cc: | Jens Arps added |
---|
comment:4 Changed 9 years ago by
Hi, my CLA was accepted.
I received email from Aimee Busch <[email protected]…> (15.nov 2011) which confirms that my Contributor License Agreement was received and my name was added to the Dojo Foundation's contributor listing.
I have been working with dojox.storage system before dojo 1.7 was released. Everything was working fine. But if you want to use it with AMD, it is not working. The reason is that storage modules have not been rewritten into new syntax and that is what I did.
comment:5 Changed 9 years ago by
OK thanks, I just don't see you on http://livedocs.dojotoolkit.org/developer/contributors, guess they forgot to add you.
I'm not sure why dojox.storage wouldn't work with AMD, in theory you can load legacy modules from an AMD modules. But anyway AMD conversion is good.
Like I said above though, those random require() calls look dodgy since they execute asynchronously, after the module has finished loading. Is there a reason you didn't list all the dependencies in the standard way?
comment:6 Changed 9 years ago by
I am there, my name is Martin Repta.
I can try to provide sample for problematic behaviour. As I remember, there was probably problem with globales used in dojox.storage.manager. I will checkout latest dojox and send the sample.
About the doggy requires, it is just an optimization, some modules are required exactly at the moment, when they are really needed (for example, see line 202 in patch file). If you wish, I can rewrite it to another version, where all modules are written in the top list.
comment:7 Changed 9 years ago by
The thing is that you've changed code that used to run synchronously to now run asynchronously, so chances are that you broke something. Like maybe an app will reference dojox.storage.manager expecting all the plugins to be registered, but they aren't registered yet because the user's callback code ran before those registerModule() calls.
comment:8 Changed 9 years ago by
Get the latest version of dojo sources from github (do not use link from CDN). Try to use storage provider with AMD in async mode. It does not work.
My patch solved the problem.
comment:9 Changed 9 years ago by
Summary: | dojox.storage in async mode → [patch] [cla] dojox.storage in async mode |
---|
comment:10 Changed 9 years ago by
Priority: | high → low |
---|
comment:11 Changed 9 years ago by
Status: | new → open |
---|
comment:12 Changed 9 years ago by
Replying to martinerko:
dojox.storage system does not work with AMD. It must to be converted into new syntax.
I did this conversion.
The built version(Which turns it to define etc.) is not working as expected , not sure how but it's broken , please fix that ASAP this holds us from moving to AMD
comment:13 Changed 9 years ago by
Milestone: | tbd → 1.8 |
---|---|
Owner: | changed from Shane O'Sullivan to dylan |
Status: | open → assigned |
comment:14 Changed 9 years ago by
First look at this patch, and it's far from complete in converting the flash module to use AMD and modules throughout. It also doesn't update the inline docs, etc.
I've started working on the conversion, but it's not a simple fix.
comment:15 Changed 9 years ago by
Milestone: | 1.8 → 2.0 |
---|
We'll need to delay this for 2.0 unfortunately.
comment:16 Changed 8 years ago by
Resolution: | → wontfix |
---|---|
Status: | assigned → closed |
dojox/storage is obsolete. The current code will not be ported to 2.0
patch file for dojox.storage system