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

Browserify support #11

Closed
lawnsea opened this issue Jan 23, 2014 · 8 comments
Closed

Browserify support #11

lawnsea opened this issue Jan 23, 2014 · 8 comments

Comments

@lawnsea
Copy link

lawnsea commented Jan 23, 2014

For jsdom/jsdom#711, I am working on making it possible to bundle jsdom with browserify. Unfortunately, that's not possible at present for this repo, becuase browserify does not attempt to understand dynamic requires like those done to pull in the property modules.

I have fixed this by expanding the array of property names into the equivalent static call for each. This is ugly as sin, but works (if you increase the maximum number of open files on OS X).

I am going to fork this repo and apply my fix, but I think the permanent fix is to have an optional build step that does the described transformation. If I build such a thing, would you consider merging it?

lawnsea referenced this issue in TreehouseJS/CSSStyleDeclaration Jan 24, 2014
Browserify doesn't support dynamic dependencies. So, replace them with static
ones. There are a lot, so you may need to up the maximum number of open files.

Workaround for chad3814/CSSStyleDeclaration#11.
@lawnsea
Copy link
Author

lawnsea commented Jan 24, 2014

Forked and pushed my workaround for this issue: TreehouseJS/CSSStyleDeclaration@9fa65f9b79

@chad3814
Copy link
Contributor

chad3814 commented Mar 3, 2014

ugh, yeah ugly, but if that's what's need, that's what's needed. I actually don't push versions from the master branch anymore, but from the 0.2.x or 0.3.x branches. I'll think a little bit about organizing this better. Maybe have a require for CSS2.1 and one for CSS3, and then break them down even more in there, for related properties.

@lawnsea
Copy link
Author

lawnsea commented Mar 3, 2014

Ok, sounds good. Let me know what you decide and I'll integrate my changes if necessary.

@domenic
Copy link
Member

domenic commented Oct 13, 2014

Would be pretty cool to somehow make this work so that we don't have to bundle the separate fork with jsdom for browserify users.

One way to do it is to create a prepublish script that generates those lines ahead of time. You can then make the pretest script be npm run prepublish and such. We did that for the Promises/A+ tests in promises-aplus/promises-tests@e099884

@chad3814
Copy link
Contributor

my mind is slow today, I'll get this fixed and still have lazy load later today

@domenic
Copy link
Member

domenic commented Oct 13, 2014

Hmm, if it can be done in an easily-maintainable way, maybe use the browser field in package.json so that browser consumers get one file and Node another?


From: chad3814mailto:[email protected]
Sent: ý2014-ý10-ý13 13:44
To: chad3814/CSSStyleDeclarationmailto:[email protected]
Cc: Domenic Denicolamailto:[email protected]
Subject: Re: [CSSStyleDeclaration] Browserify support (#11)

actually, the problem is now it lazy loads the files because of #14https://github.com/chad3814/CSSStyleDeclaration/pull/14 I'm not sure what I can do make both happy...


Reply to this email directly or view it on GitHubhttps://github.com/chad3814/CSSStyleDeclaration/issues/11#issuecomment-58928480.

chad3814 added a commit that referenced this issue Oct 13, 2014
This file will lazy-load properties but also specify the filename in the
require explicitly so that it plays better with browserify.

Should solve #11
@chad3814
Copy link
Contributor

% npm publish

[email protected] prepublish .
node ./scripts/generate_properties.js

npm http PUT https://registry.npmjs.org/cssstyle
npm http 201 https://registry.npmjs.org/cssstyle

@chad3814
Copy link
Contributor

% npm publish

[email protected] prepublish .
node ./scripts/generate_properties.js

npm http PUT https://registry.npmjs.org/cssstyle
npm http 201 https://registry.npmjs.org/cssstyle

chad3814 added a commit that referenced this issue May 13, 2015
This file will lazy-load properties but also specify the filename in the
require explicitly so that it plays better with browserify.

Should solve #11
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

No branches or pull requests

3 participants