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.

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

Refresh targets ignores .atom-build.json #527

Open
wiggisser opened this issue Jul 14, 2017 · 6 comments
Open

Refresh targets ignores .atom-build.json #527

wiggisser opened this issue Jul 14, 2017 · 6 comments

Comments

@wiggisser
Copy link

When I add a new target to my .atom-build.json and then do "Packages -> Build -> refresh targets" the new target does not appear in the list of available targets. I have to restart atom to have those changes reflected. Same for removing a target or updating its cmd

@noseglid
Copy link
Owner

noseglid commented Oct 2, 2017

Atom build is actively watching the build file so a refresh target shouldn't be necessary.
That it doesn't even work when you're issuing "refresh targets" (which is a manual way of doing what should be automatic) is weird.

Are you getting any errors in the console or something like that?

Might add that I am unable to reproduce, so there's likely something else at play here.

@wiggisser
Copy link
Author

I did open the developer tools (Ctrl+Shift+I) and ran a refresh targets. Did not show any additional message at all. I set a breakpoint in the refreshtargets function and the breakpoint got hit shortly after I saved changes to the file, so the watch mechanism seems to work.

I then tried to dig deeper and came to the point where the actual json file is loaded, the function getConfig(file) in atom-build.js. And this seems to be the problem: Whereas in my file in the editor I only have two targets defined, the require returns an object which contains 4 targets. It seems, this always returns the inital state of the file, when it was first required at the atom start and does not reread the file after it was changed.

I'm currently running atom 1.20 on linux mint, but I noticed this issue on all versions of atom since I created this issue, and also on Windows 8.1

@wiggisser
Copy link
Author

wiggisser commented Oct 2, 2017

I replaced require with readfile and parse for JSON file. This works for me, and refreshes the targets.

I'm pretty sure, this issue applies for .js files too, but I didn't touch anything there.
See also this stackoverflow thread on require and its cache
https://stackoverflow.com/questions/9210542/node-js-require-cache-possible-to-invalidate

Sorry, didn't see, you already do a delete on the require cache, but that doesn't seem to work (at least for json files, did not try js)

@scratchboom
Copy link

As a workaround, I just rename .atom-build.json to .atom-build.cson. json and cson are compatible (at least for my simple .atom-build file)

@jmariner
Copy link

jmariner commented Feb 17, 2018

I investigated this problem since I had a similar issue with a package I'm writing. It turns out that Atom uses its own require cache and clearing the normal require.cache does not affect the custom one.

The custom require cache is at snapshotResult.customRequire.cache (snapshotResult is a global). The paths within are all relative to [atom installation dir]/app-x.xx.x/resources/app.asar/static/, so I used the following to build this path and remove it from the cache:

const path = require("path");
const originalPath = "absolute/path/to/file";

const cachePath = path.relative(
    process.resourcesPath + 
        // these directory names don't actually matter, could be "/a/b" instead
        "/app.asar/static",
    originalPath
).replace(/\\/g, "/"); // custom cache forces "/" as the path separator

delete snapshotResult.customRequire.cache[cachePath];

This would probably be the better approach to fixing this instead of manually reading and parsing the files, but may break in the future. Ideally, Atom needs to implement a concrete way to clear the require cache since it's very common for packages like this one to need to reload required JS or JSON files.

Edit: I just found that this was somewhat discovered (#542 (comment)), but no full fix was created. I can turn this into a PR to fix this if needed.

@wiggisser
Copy link
Author

wiggisser commented Feb 17, 2018

@jmariner Thanks for your comment. I already figured this out quite a while ago (see the comments in #542 for details) long story short: atom won't change the current behavior. Thus all packages requiring module reload need either an alternative loading mechanism (for instance like in #553) or force a restart of atom.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants