Skip to content
This repository was archived by the owner on Sep 6, 2021. It is now read-only.

Conversation

@zaggino
Copy link
Contributor

@zaggino zaggino commented Nov 23, 2016

PR for #12937

@zaggino zaggino mentioned this pull request Nov 23, 2016
@ficristo
Copy link
Collaborator

ficristo commented Dec 1, 2016

Why not use requirejs to create some logical mapping between the node_modules and thirdparty folders?
I think @petetnt has done that in #12006
If we go on this route we probably need to do something for distribution. (I don't remember exactly but I think we don't copy the node_modules over there)

@zaggino
Copy link
Contributor Author

zaggino commented Dec 1, 2016

Not sure how to do any logical mapping here, but happy to learn. We could use a separate package.json file inside the src folder for production dependencies. That's how brackets-electron does it and that's how electron authors recommend electron projects to be done.

@ficristo
Copy link
Collaborator

ficristo commented Dec 1, 2016

The other idea I had in mind was exactly putting a package.json inside the src folder.
Both cases seems better to me.
For the logical mapping @petetnt knows more.

@petetnt
Copy link
Collaborator

petetnt commented Dec 1, 2016

One can use relative mapping with ../node_modules/... for the requirejs config so you can use deps from the root folder: https://github.com/petetnt/brackets/blob/petetnt/poc-thirdparty-with-npm/src/main.js#L68. Not sure if that works in browser though.


// task: install
grunt.registerTask('install', ['write-config', 'less', 'npm-install-extensions']);
grunt.registerTask('install', ['write-config', 'less', 'copy-browser-dependencies', 'npm-install-extensions']);
Copy link
Contributor

Choose a reason for hiding this comment

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

Fast note: the script is referenced here as copy-browser-dependencies but is actually named move-browser-dependencies.

@zaggino
Copy link
Contributor Author

zaggino commented Dec 12, 2016

Closing in favor of #12972

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants