[#CLJS-129] Loading namespaces from the ClojureScript REPL does not always work - Clojure JIRA

Details

Type: Defect
Status: Resolved
Priority: Minor
Resolution: Completed
Affects Version/s: None
Fix Version/s: None
Component/s: None
Labels:
None

Description

The following example does not work as expected when run form a ClojureScript REPL:

(ns test.color (:require [goog.color :as c]))
(c/parse "#000000")

The parse function is undefined.

The problem is that goog.color depends on goog.color.names. goog.color.names is loaded first and goog.color is never loaded because the current implementation assumes that if goog.color is not undefined then it has already been loaded.

Come up with a different strategy for determining if a namespace is loaded.

More Options
- Sort By Name
- Sort By Date
- Ascending
- Descending
- Download All

Attachments

129-repl-code-loading.patch

21/Jan/12 3:51 PM

3 kB

Brenton Ashworth

Activity

Ascending order - Click to sort in descending order

Hide

Permalink

Brenton Ashworth added a comment - 21/Jan/12 3:59 PM

This patch removes the browser check for loaded namespaces which can return a false positive. This fixes the problem. With this change, namespaces which are loaded by the browser may be re-loaded from the REPL one time. From the REPLs perspective each namespace is loaded only once.

Show

Brenton Ashworth added a comment - 21/Jan/12 3:59 PM This patch removes the browser check for loaded namespaces which can return a false positive. This fixes the problem. With this change, namespaces which are loaded by the browser may be re-loaded from the REPL one time. From the REPLs perspective each namespace is loaded only once.

Hide

Permalink

David Nolen added a comment - 19/Apr/12 11:51 PM

Is this fixed?

Show

David Nolen added a comment - 19/Apr/12 11:51 PM Is this fixed?

Hide

Permalink

Brenton Ashworth added a comment - 20/Apr/12 3:26 PM

This is not fixed. This patch solves the problem but may cause others. I am still thinking about this one and haven't forgot about it.

This problem only occurs in the situation described above when evaluating ns forms in a browser-connected REPL.

The problem is: namespaces may be loaded by the browser or sent to the browser from the REPL when evaluating a ns form. The REPL keeps track of which namespaces have been sent to the browser so that it doesn't resend them unnececcarily. But the REPL is not aware of which namespaces were loaded by the browser. Before sending a namespace to be evaluated in the browser, a check is performed to see if this namespace already exists. This check does not always work and may indicate that a namespace exists which has not actually been loaded (see the example above). I don't know of a reliable way to check for this so this patch simply removes the check. This means that every time a new REPL is started, evaluating a ns form will cause all of the dependencies for that namespace to be re-evaluated in the browser one time.

If we think that this behavior is acceptable then we can use this patch. Otherwise, we need to find some way to know what has already been loaded in the browser.

Show

Brenton Ashworth added a comment - 20/Apr/12 3:26 PM This is not fixed. This patch solves the problem but may cause others. I am still thinking about this one and haven't forgot about it. This problem only occurs in the situation described above when evaluating ns forms in a browser-connected REPL. The problem is: namespaces may be loaded by the browser or sent to the browser from the REPL when evaluating a ns form. The REPL keeps track of which namespaces have been sent to the browser so that it doesn't resend them unnececcarily. But the REPL is not aware of which namespaces were loaded by the browser. Before sending a namespace to be evaluated in the browser, a check is performed to see if this namespace already exists. This check does not always work and may indicate that a namespace exists which has not actually been loaded (see the example above). I don't know of a reliable way to check for this so this patch simply removes the check. This means that every time a new REPL is started, evaluating a ns form will cause all of the dependencies for that namespace to be re-evaluated in the browser one time. If we think that this behavior is acceptable then we can use this patch. Otherwise, we need to find some way to know what has already been loaded in the browser.

Hide

Permalink

David Nolen added a comment - 20/Apr/12 4:59 PM - edited

The browser REPL could take a (optional?) source path to analyze before starting up right? My experience is that analysis is pretty quick even for many files.

Show

David Nolen added a comment - 20/Apr/12 4:59 PM - edited The browser REPL could take a (optional?) source path to analyze before starting up right? My experience is that analysis is pretty quick even for many files.

Hide

Permalink

Brenton Ashworth added a comment - 20/Apr/12 7:05 PM

Yes. I think that might be the way to go. I'll give that a try and try to get this issue resolved by next week.

Show

Brenton Ashworth added a comment - 20/Apr/12 7:05 PM Yes. I think that might be the way to go. I'll give that a try and try to get this issue resolved by next week.

Hide

Permalink

Brenton Ashworth added a comment - 08/Jun/12 2:38 PM

This is fixed with commit ba8a1f57.

The browser-connected REPL keeps track of all of the namespaces it has loaded and will only load them once. Some namespaces will have already been loaded when the host page was loaded in the browser. This fixes how we attempt to determine what has already been loaded. Instead of asking the browser what has been loaded, which is not reliable, we allow namespaces to be loaded a second time. If loading a namespace a second time will cause problems, the namespace can be passed to repl-env to be ignored.

See the doc string for cljs.repl.browser/repl-env for more information.

Many namespaces are loaded in the process of running a browser-connected REPL. These namespaces will always be ignored.

We can't determine what has been loaded in the browser by analyzing source code because there may be sources that, for some reason, have not been sent to the browser.

Show

Brenton Ashworth added a comment - 08/Jun/12 2:38 PM This is fixed with commit ba8a1f57. The browser-connected REPL keeps track of all of the namespaces it has loaded and will only load them once. Some namespaces will have already been loaded when the host page was loaded in the browser. This fixes how we attempt to determine what has already been loaded. Instead of asking the browser what has been loaded, which is not reliable, we allow namespaces to be loaded a second time. If loading a namespace a second time will cause problems, the namespace can be passed to repl-env to be ignored. See the doc string for cljs.repl.browser/repl-env for more information. Many namespaces are loaded in the process of running a browser-connected REPL. These namespaces will always be ignored. We can't determine what has been loaded in the browser by analyzing source code because there may be sources that, for some reason, have not been sent to the browser.

People

Assignee:

Brenton Ashworth

Reporter:

Brenton Ashworth

Vote (0)

Watch (1)

Dates

Created:

20/Jan/12 2:32 PM

Updated:

08/Jun/12 2:38 PM

Resolved:

08/Jun/12 2:38 PM