Skip to content

#139 error can't resolve 'fs' fix - #146

Closed
IanGrainger wants to merge 2 commits into
reworkcss:masterfrom
IanGrainger:master
Closed

IanGrainger wants to merge 2 commits into
reworkcss:masterfrom
IanGrainger:master

Conversation

@IanGrainger

Copy link
Copy Markdown

Catch error if fs module not found - as when running in a browser.

This means that parse will work - which is all I need. Though it won't allow you to use sourcemaps!

try {
var fs = require("fs");
} catch (e) {
var fs = null;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

There should be check for fs !== null in all methods, that depends on it. In case fs is not loaded it should throw some error to make it clear.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I think I've done that - there's only one place that needs changing for runtime I think?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I've now created a second PR with an alternative approach - I think I like it a little more - but up to you! 😄

#147

@lr-mjaouen lr-mjaouen Oct 26, 2020 •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I'm not sure to understand but if it's a dep, why to not add it to the dependencies or peers first?

@IanGrainger IanGrainger Oct 26, 2020 •

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I'm not sure I know what you mean. There is no equivalent to fs in the browser. But with this workaround I can parse CSS in the browser... Meaning I get to use the jasmine-dom library from Karma! 😍 So, what needs to change? 🙏

@IanGrainger IanGrainger changed the title Workaround for issue #139 #139 error can't resolve 'fs' fix Oct 26, 2020
@IanGrainger

Copy link
Copy Markdown
Author

Is this repo active? I created PRs a week ago... 😦

@IanGrainger

Copy link
Copy Markdown
Author

Is this repo active? I created PRs a week ago... 😦

2 weeks ago now... @ogonkov / @lr-mjaouen - any advice on how I can get this (or the other PR) merged would be greatly appreciated 🙏

@Fi1osof

Fi1osof commented Nov 19, 2020

Copy link
Copy Markdown

Project dead?

@IanGrainger

Copy link
Copy Markdown
Author

Project dead?

Yeah. Seems like it 😦 .

Its obviously good enough at what it does - because there's no alternatives that I can see, either!

@charick

charick commented Nov 29, 2020

Copy link
Copy Markdown

facing the same problem..

@Fi1osof

Fi1osof commented Dec 8, 2020

Copy link
Copy Markdown

@IanGrainger seems like this PR does not make sense. Please have a look index file: https://github.com/reworkcss/css/blob/master/index.js

exports.parse = require('./lib/parse');
exports.stringify = require('./lib/stringify');

Just use

// Can not use import cssParse from 'css/lib/parse' due @types/css does not have correct declarations
const cssParse = require('css/lib/parse');
// ....
const css = cssParse(inputCssText);

instead of

import cssParser from 'css';
// ...
var css = cssParser.parse(inputCssText);

In this case you import only css parser without stringify.
This works for me.

@IanGrainger

IanGrainger commented Dec 8, 2020 •

Copy link
Copy Markdown
Author

Just use

// Can not use import cssParse from 'css/lib/parse' due @types/css does not have correct declarations
const cssParse = require('css/lib/parse');
// ....
const css = cssParse(inputCssText);

In this case you import only css parser without stringify.
This works for me.

Um, I tried that and got cssPase is not a function.

const { cssParse } = require('css/lib/parse');

function parseCSS(css, ...args) {
  const ast = cssParse(`selector { ${css} }`, {
  silent: true,}).stylesheet;
  ...

@Fi1osof

Fi1osof commented Dec 8, 2020

Copy link
Copy Markdown

Um, I tried that and got cssPase is not a function.

Not

const { cssParse } = require('css/lib/parse');

But

const cssParse = require('css/lib/parse');

@IanGrainger

Copy link
Copy Markdown
Author

Um, I tried that and got cssPase is not a function.

Not

const { cssParse } = require('css/lib/parse');

But

const cssParse = require('css/lib/parse');

Argh. I fail at copy+paste, sorry! You're right!!

@IanGrainger

Copy link
Copy Markdown
Author

I've created a PR on the project I'm trying to fix. Thank you @Fi1osof !! testing-library/jasmine-dom#8

@Fi1osof

Fi1osof commented Dec 9, 2020

Copy link
Copy Markdown

I've created a PR on the project I'm trying to fix. Thank you @Fi1osof !! testing-library/jasmine-dom#8

Not at all! :)

@ibalaji777

Copy link
Copy Markdown

i hide fs in my source now its started working but i just use string css so i think i don't want fs

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.

6 participants