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

CodeMirror update#12940

Closed
zaggino wants to merge 3 commits into
masterfrom
zaggino/npm-codemirror
Closed

CodeMirror update#12940
zaggino wants to merge 3 commits into
masterfrom
zaggino/npm-codemirror

Conversation

@zaggino

@zaggino zaggino commented Nov 23, 2016

Copy link
Copy Markdown
Contributor

PR for #12937

@zaggino zaggino mentioned this pull request Nov 23, 2016
@ficristo

ficristo commented Dec 1, 2016

Copy link
Copy Markdown
Collaborator

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

zaggino commented Dec 1, 2016

Copy link
Copy Markdown
Contributor Author

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

ficristo commented Dec 1, 2016

Copy link
Copy Markdown
Collaborator

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

petetnt commented Dec 1, 2016

Copy link
Copy Markdown
Collaborator

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.

Comment thread Gruntfile.js

// 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
Copy Markdown
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

zaggino commented Dec 12, 2016

Copy link
Copy Markdown
Contributor Author

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.

4 participants