Skip to content

Save instructions.json in root dir#95

Merged
bajtos merged 1 commit intostrongloop:masterfrom
r3dm:fix/save-instruct-in-root
Feb 20, 2015
Merged

Save instructions.json in root dir#95
bajtos merged 1 commit intostrongloop:masterfrom
r3dm:fix/save-instruct-in-root

Conversation

@BerkeleyTrue
Copy link
Copy Markdown

Saving instructions in node_modules dir causes complaints and missing files
fixes #94

@slnode
Copy link
Copy Markdown

slnode commented Feb 18, 2015

Can one of the admins verify this patch? To accept patch and trigger a build add comment ".ok\W+to\W+test."

@bajtos
Copy link
Copy Markdown
Member

bajtos commented Feb 18, 2015

Thank you for the pull request. The reason why we need to write the instructions to a file is that at the time of implementing the browserify feature, there was a bug browserify/browserify#771 which prevented us from using an in-memory stream. The bug should have been fixed in 5.x, thus we should be able to use a stream now - see the commented out code on L86-L92 of lib/bundler.js.

Could you please upgrade browserify to the latest version and check if the stream-based solution works now? If it does, then I'd much rather drop the hack (writing the file) instead of trying to fix it.

@BerkeleyTrue
Copy link
Copy Markdown
Author

Just did a test run with Browserify 6.3 and it is working. Should I just amend this PR?

@bajtos
Copy link
Copy Markdown
Member

bajtos commented Feb 18, 2015

Just did a test run with Browserify 6.3 and it is working.

Great!

Should I just amend this PR?

Yes please.

@BerkeleyTrue
Copy link
Copy Markdown
Author

Should I also update devDep browserify version to latest?

@bajtos
Copy link
Copy Markdown
Member

bajtos commented Feb 18, 2015

Should I also update devDep browserify version to latest?

Update it to ^x.y.z where x.y.z is the current latest version that works for you.

@BerkeleyTrue
Copy link
Copy Markdown
Author

While I can confirm it gives me the correct compile in my app, I can't get two tests (browser.test.js fails while all other test are green) to pass using stream method. It seems to hang waiting for browserify to close the pipe. Can't see why, but my knowledge of pipes here is lacking. How should I proceed?

@bajtos
Copy link
Copy Markdown
Member

bajtos commented Feb 19, 2015

I don't have time now to investigate this myself. Let's stick to the workaround solution we have now then.

lib/bundler.js Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am little bit worried about adding a generated file that looks like it is a part of loopback-boot sources. Can we use a different name that would make it clear that the file is a temporary artefact created during the build? Few suggestions: .generated-instructions.js or .browser.instructions.js.

Saving in node_modules dir causes complaints and missing files
fixes #94
@BerkeleyTrue
Copy link
Copy Markdown
Author

@bajtos file now saving as generated-instructions.json

bajtos added a commit that referenced this pull request Feb 20, 2015
Save instructions.json in root dir
@bajtos bajtos merged commit 9b407b1 into strongloop:master Feb 20, 2015
@bajtos
Copy link
Copy Markdown
Member

bajtos commented Feb 20, 2015

Landed, thank you!

@bajtos
Copy link
Copy Markdown
Member

bajtos commented Feb 20, 2015

Released as loopback-boot@2.6.5.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: saving instructions to node_modules requires folder with package.json

4 participants