Deleting all modules from library causes error in editor

VERIFIED FIXED in Builder 0.9.8

Status

addons.mozilla.org Graveyard
Add-on Builder
P1
major
VERIFIED FIXED
7 years ago
3 years ago

People

(Reporter: stephend, Assigned: seanmonstar)

Tracking

unspecified
Builder 0.9.8

Details

(Reporter)

Description

7 years ago
STR:

1. Using Firefox 4.01, log in to https://builder-addons.allizom.org/
2. Click "Create" and then choose "Library"
3. Delete the index.js file and once that's removed, type in the editor widget

Actual results:

Error: item is null
Source File: https://builder-addons.allizom.org/media/lib/mootools/mootools-core-1.3.2.js
Line: 936

Screencast: http://screencast.com/t/NBtsf4aKDh

Updated

7 years ago
Target Milestone: Builder 0.9.6 → Builder 0.9.7
(Assignee)

Comment 1

7 years ago
So, when you delete all files, the editor window should become read-only, until a new file is created.
Assignee: nobody → smcarthur
Priority: -- → P1
Can we just prevent rename/delete of the index.js file as we do with main.js?
(Assignee)

Comment 3

7 years ago
If we want to enforce that all Libraries have an index.js. It wouldn't change names (otherwise, its too hard to realize why one of your modules can't be deleted). 

We could then add to the package.json of the library that the index.js is the main_module. This would allow require('libname'), and it would load that main_module of that library (currently set to nothing, but could be the index.js file).
Yeah that sounds good, I especially like this part: "This would allow require('libname')". Let's do it, you agree?
I believe the best would be a library to have at least one module
(Assignee)

Comment 6

7 years ago
An example of the downside: your mootools-server has its module also named mootools-server.

If we enforced a required index.js, we would need to add one to any library that doesn't have one. Then, whereas before when require('mootools-server') would first try to load the library, see there was no main_module, and then look for modules and find 'mootools-server', with this it would load the index.js, which could have nothing in it.

Still would be better to make this "breakage" now as opposed to later...
++ as I said - prevent (in JavaScript only) deletion of the last module in library.
(Assignee)

Comment 8

7 years ago
So which is it? All libraries get a permanent index.js module (allowing require('mylib'), but breaking some pre-existing uses), or simply stop deleting modules if module count == 1?
I'd vote for the latter

Updated

7 years ago
Target Milestone: Builder 0.9.7 → Builder 0.9.8
(Assignee)

Comment 10

7 years ago
I personally vote for the former, because I think being able to require('mylib') to be a beneficial pattern for the Builder.

dbuc, which?
I'd like to see the former, this needs to work: require('mylib');
(Assignee)

Updated

7 years ago
Summary: JS Error in mootools.js - Error: item is null → Deleting all modules from library causes error in editor
(Assignee)

Comment 12

7 years ago
Code has landed. https://github.com/mozilla/FlightDeck/commit/53866e1867bade9ab95292a7f03e8bb0ba6295e2

However, the migration fails. @clouserw: what should i change to get this working?

Updater output:

[localhost] out: Running .py migation 11:
[localhost] out: python -B manage.py runscript migrations.011-ensure_library_main_module
[localhost] out: Error: Had trouble running this: python -B manage.py runscript migrations.011-ensure_library_main_module
[localhost] out: stdout: Unknown option: -B
[localhost] out: usage: python [option] ... [-c cmd | -m mod | file | -] [arg] ...
[localhost] out: Try `python -h' for more information.
(Assignee)

Comment 13

7 years ago
Got the migration working. Fixed on master.
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
(Reporter)

Comment 14

7 years ago
Verified FIXED; unable to get UI to delete index.js under "Lib" in a library.
Status: RESOLVED → VERIFIED
Product: addons.mozilla.org → addons.mozilla.org Graveyard
You need to log in before you can comment on or make changes to this bug.