Skip to content
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.

Sign up for GitHub

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

Jump to bottom

disentangle js files ("modules") #80

Closed
unhammer opened this issue Oct 22, 2016 · 12 comments
Closed

disentangle js files ("modules") #80

unhammer opened this issue Oct 22, 2016 · 12 comments
Labels
enhancement tech-debt

Comments

@unhammer
Copy link
Member

Currently, there's a lot of interdependency in the js files, e.g. localisation depends on stuff from util that depends on stuff from localisation. It'd be nice if the dependencies were hierarchical (no cycles), to make it possible to use e.g. http://flowtype.org/ on the individual files instead of just on the concatenation of the whole project :)
@unhammer unhammer mentioned this issue Feb 3, 2017
Closed
@sushain97 sushain97 mentioned this issue Mar 12, 2017
Closed
@unhammer unhammer mentioned this issue Jun 4, 2017
Closed
@Androbin
Copy link
Contributor

The only real dependency cycle exists between translator.js and util.js.
First, we should probably figure out where this block in util.js should go. This code clearly doesn't belong in the utility file.

@Androbin
Copy link
Contributor

This would resolve the dependency cycle without further ado. In fact, everything that util.js imports from persistence.js, localization.js and translator.js is fully contained within this block.

@sushain97
Copy link
Member

sushain97 commented Dec 16, 2017 •
edited
Loading

First, we should probably figure out where this block in util.js should go. This code clearly doesn't belong in the utility file.

Maybe init.js? Any other suggestions?

More importantly, is there a way to prevent this from re-emerging as an issue? Perhaps build flow into the build system (i.e. CircleCI)?

@Androbin
Copy link
Contributor

More importantly, is there a way to prevent this from re-emerging as an issue? Perhaps build flow into the build system (i.e. CircleCI)?

Maybe we can enforce constraints on which modules may import certain other modules.

@Androbin Androbin mentioned this issue Dec 16, 2017
Merged
@sushain97
Copy link
Member

Maybe we can enforce constraints on which modules may import certain other modules.

How? I'd much rather just run Flow... then we get type errors reported as well. (since this task otherwise is literally just copy pasting code, this should get done as well).

@Androbin
Copy link
Contributor

Why do only translator.js and util.js contain the @flow annotation?

@sushain97
Copy link
Member

Why do only translator.js and util.js contain the @flow annotation?

I dunno. @unhammer did it ;) Presumably, the annotation can be added to all files without actually adding any types yet.

@unhammer
Copy link
Member Author

Why do only translator.js and util.js contain the @flow annotation?

I guess those are the files I worked most on :) Or maybe the other files gave too many errors at the time.

@Androbin
Copy link
Contributor

Androbin commented Dec 17, 2017 •
edited
Loading

@unhammer So, shall we add the annotation to the other files as well (in #229)?

@unhammer
Copy link
Member Author

Yes, if it doesn't break anything (or if you fix what it breaks ;))

@Androbin
Copy link
Contributor

Spoiler: It breaks.

@sushain97
Copy link
Member

(Flow removed weak mode so now we have to explicitly type a bunch of stuff for it to even start working...)

sushain97 pushed a commit that referenced this issue Jan 4, 2018
@Androbin @sushain97
Merge branch 'issue-55-suggestions' into master (#249)
854ca0c 
* Add file download workaround for Safari (fixes #97)

* Changed Download_Browser_Warning

* Added workaround for Safari

Fixed translator.js

Fixed wierd bugs that I have no idea how I had made.

* Moved .fadeIn out of conditional block

* Added comments to increase code readability

* Allow right-click to download even if workaraound is used

* Revert string changes

* Fix multiple download in Safari >10

Fix es-lint

Fix es-lint

* Removed Download_Browser_Warning

* Change way we check for Safari version

Instead of looking at specific version (10, 11) look for one digit version (9, 8 and earlier)

* Remove unused global varaibles

* Extended comment about Safari workaround

* Run cleanup

* Fixes #201 (Chrome audit improvements)

* Improved Chrome audit performance

* Update Makefile

* Fixed formatting errors

* Fixed more formatting errors

* Revised "start_url"

* Removed sw.js

* Removed script for registering service worker

* Revised makefile, manifest.json, and index.html

* Improved Chrome audit performance further

* Improved audit performance further

* Fixed errors

* fixed houndci error

* Fixed final errors

* Added whitespace

* Fix <html> localisation

* Disentangle JS modules and fix Flow (fixes #80)

* [WIP] Disentangle JS modules (#225)

* Disentangle JS modules

* Fix imports

* Use flow on individual files

* Declare modules to Flow

* Run Flow directly

* Cleanup

* Fix some import/export issues

* Make the build work

* Tweak flow decl

* Move around the config decl to make build work

* Delete extra line

* Fix make debug (fixes #220)

* Improvements to Flow coverage

* Better flow coverage

* Doesn't look like the stubs actually belong to jQuery 3 so I'm reverting these changes

* Remove unnecessary var

* This name makes a lot more sense

* Remove unnecessary type annotations (clutter code & are obvious e.g. string literals)

* More unnecessary type annotations

* Remove more unnecessary annotations

* Prefer literal union type

* Add a type and remove one

* More fixes

* Fix persist choices

* Remove more redundant annotations

* Remove more unnecessary annotations

* Switch to Array<...> shortform

* Undo one of my 'fixes'

* Final flow fix

* Convert Object to {} shorthand

* Remove a couple more unnecessary annotations

* Some final cleanup

* Implement requested changes

* Add/Remove/Fix types

* Adapt config types

* Un-Flow strict.js

* Allow subtitle to be null

* Final tweaks

* Simplify progress bar update

* Split CSS into multiple files and improve organization (fixes #212)

* Split CSS into multiple files

* Better organization

* Minor tweaks

* Bring all the coverage over 90%

* Fix buggy UI with detect-language (fixes #236)

* Refactor repeated code into a function

* Fix the locale selector I probably broke

* Show translation not available less often

* Remove a warn that gets triggered all the time

* More generic port configuration with Docker

* Less awkward input label placement

* Slightly more sane language sorting with variants

* Significantly more sane translation language column computation

* Fix typo

* Store docker pair data in persistent volume and add update all script

* Include language modules download in deploy script (fixes #244)

* Minify JS and CSS (towards #130)

* Minify all.js and all.css

* Make minification fail gracefully

* Update README section on compression

* Use command line interfaces

* Update Dockerfile

* Update README.md

* Update README.md

* Fix dockerfiles

* Fix debug HTML

* Fetch monolingual modules before pairs in deploy-all script

* Merge handleTranslateSuccessResponse

* Update imports/exports

* Add JQuery modal/tooltip stubs

* Add config.SUGGESTIONS stubs

* Fix remaining Flow issues
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement tech-debt
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@unhammer @sushain97 @Androbin