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

Including 'sass.sync.js' in Bower #32

Closed
wants to merge 1 commit into from

Conversation

Wildhoney
Copy link

Including the dist/sass.sync.js in the Bower registry. I realise it's not recommend for production, but in my case I want to provide basic inline SASS support for development – when working with my newly created Maple framework. Alternatively I could use the WebWorker but for development the configuration path required is an additional complication, and something I'd need to document. Using dist/sass.sync.js the developer merely needs to include it as a script, as opposed to configuring Maple 👍

@rodneyrehm
Copy link
Member

Thank you for your effort and interest in this. I am very reluctant to making this happen, because the sync module is the wrong approach for a browser. It can be the equivalent of while(true). It can block rendering and user interaction. It is not your friend. You really want to focus on making the worker usable easily.

If you can live with the user having to provide dist/sass.sync.js so it can find dist/libsass.js.mem, then you could also just hard-code the path to sass.worker.js - I don't see much of a difference there.

@Wildhoney
Copy link
Author

Agreed, and I would also be reluctant to hog the main event loop. With the sync approach, I was simply going to check for Sass#compile's existence. If I were to take the worker approach, then I'd need to add a little Sass.js specific configuration in Maple to specify the path – it cannot be hard-coded because it's a framework (I use this term loosely, as it's simply a library to enable webcomponents in React.js).

Maple also doesn't expose any public API — there's no window.maple object. My other idea was allowing the developer to specify the path on the script tag for Maple, but this seems far from ideal:

<script type="text/javascript" src="vendor/maple/maple.js" data-sass="vendor/sass/sass.js"></script>

Thus you can see why checking for Sass#compile would be the simplest approach – ignoring any obvious downsides such as blocking.

I'd really appreciate some suggestions on how to tackle this!

Edit: Would also like to point out that this would be purely for development where blocking code is not necessarily the end of the world.

@rodneyrehm
Copy link
Member

My other idea was allowing the developer to specify the path on the script tag for Maple, but this seems far from idea

Sass.js did extactly that in the very beginning. in 0.9.0 the API will change to require you to actually instantiate a new Sass instance var sass = new Sass('path/to/sass.worker.js') because you may run multiple instance in parallel. You may also want to defer the actual loading of the worker. That's why I did away with the magic automations.

… that this would be purely for development …

When has such a limitation ever stopped someone from pushing things into production?

I'd really appreciate some suggestions on how to tackle this!

I wrote about Identifying the current <script> element which may give you an idea for simple configuration…

If things are development only, and you know the environment (e.g. use of bower is required), then you should also know the path from where to load, no? You may try using the most common path to load from and allow the user to change that configuration where needed. That may keep it simple for the 80%, but not screw the other 80% ;)

@Wildhoney
Copy link
Author

Thanks – will have a think about this.

I personally always change from the default bower_components directory, as I'm sure other people do, too. Probably I'll go down the path of specifying a data-* attribute, as exposing the window.maple object seems unnecessary for a simple configuration parameter.

@Wildhoney Wildhoney closed this May 18, 2015
@Wildhoney
Copy link
Author

What's your position on having Sass.js attempting to resolve its worker? Essentially that's what I'll be doing. However, in my case there will be two incertitudes:

  • That I have the correct sass.js;
  • That the worker resides in the expected location;

Whereas if Sass.js were to assume a default if you write var sass = new Sass(); then only one uncertainty exists – the latter.

@rodneyrehm
Copy link
Member

I don't understand where your first uncertainty comes from


Not wanting to dictate anything, Sass.js made this part configurable. I'm not sure how I would resolve anything without knowing the exact setup the user has. In my case for example, sass.js is pulled in via require.js and dumped into a file with everything else - except for the worker. The worker is moved somewhere else during build, than it was pulled from in development mode.

But I am open to suggestions and discussion.

@Wildhoney
Copy link
Author

First uncertainty comes from not being able to reliably locate the script element using document.currentScript – as far as I see it, I'd have to iterate over the script elements and match /sass\.js/dist/sass\.js/i – not a huge uncertainty, but a greater uncertainity than having sass.js resolve its own script tag.

In my case, I'm trying to transparently use Sass.js with the least amount of configuration for the developer. In your case — using Require — an assumed default won't work, but would work for a good portion of developers who are adding Sass.js via a script tag, and who haven't moved the worker elsewhere.

In that case, we could resolve the path that sass.js resides in, and use that as a default if no path has been explicitly specified in the constructor:

constructor(workerPath) {
     var worker = new WebWorker(workerPath || defaultWorkerPath);
}

If both workerPath and defaultWorkerPath are null then the default behaviour of not being able to find the worker is maintained. Likewise if either are invalid paths.

I realise it's quite specialised and so unlikely to be included as part of Sass.js and that I'll have to find a better workaround, but I thought I'd put the idea out there.

@rodneyrehm
Copy link
Member

I'm not happy with how things are right now. Littering <script> tags all over the place is certainly a possibility, but one I wouldn't "optimize" for. That said, if you're using a tool to consume AMD or CommonJS, chances are you're doing a lot of fancy things in a build step. That means no "best guess" scenario would ever work anyway. How about appending the following to sass.js?

// we can only run this test in the browser,
// so make sure we actually have a DOM to work with
if (typeof document !== 'undefined' && document.getElementsByTagName) {
  var currentScript = document.currentScript || (function() {
    var scripts = document.getElementsByTagName('script');
    return scripts[scripts.length - 1];
  })();

  var defaultWorkerUrl = currentScript.src;
  // make sure we're not running in some concatenated thing
  if (defaultWorkerUrl.slice(-8) === '/sass.js') {
    // this may look ugly, but the browser will resolve that path before talking to a server
    Sass.initialize(defaultWorkerUrl + '/../sass.worker.js');
  }
}

That should work for any scenario loading the unmodified Sass.js in <script>, where the entire dist/ directory is maintained somewhere. Again, not how I would like to use sass.js, but then who am I to decide such things for anyone else.

@rodneyrehm
Copy link
Member

tested it, liked it, will commit it after dinner :)

@Wildhoney
Copy link
Author

👍 Any luck?

@rodneyrehm
Copy link
Member

writing testing and committing, yes. pushing to github, no. my hotel's wifi is weird that way. it has no problem with torrent, but don't you dare using SSH…

@rodneyrehm
Copy link
Member

… If you try often enough… successfully pushed to master. As I haven't release 0.9.0 yet, you'd need to build sass.js locally to test. How quickly do you need this to be available through regular channels?

@Wildhoney
Copy link
Author

Thanks! No problem – I can point to the master branch via Bower until you release a tagged version.

@rodneyrehm
Copy link
Member

actually you can't, as the dist files are only updated for a release. If I update them, I might as well release it properly… I was hoping to land Custom Functions with 0.9.0, though…

@Wildhoney
Copy link
Author

Ah! In that case, it would be lovely to have it as soon as possible – although I'm happy to build manually for the time being if you want to hold off on the minor increment. I think it all depends on when you're expecting custom functions to arrive.

@rodneyrehm
Copy link
Member

If I can't land it before Friday, I'll release 0.9.0 without it, ok?

@Wildhoney
Copy link
Author

Purrrfect 🐱 Thanks for your help, Rodney!

Wildhoney added a commit to Wildhoney/Maple.js that referenced this pull request May 19, 2015
@rodneyrehm
Copy link
Member

I didn't get the chance to work on the custom functions stuff, so I released 0.9.0 a moment ago. Thank you for your support!

@Wildhoney
Copy link
Author

Ah, nice! Thanks! 👍

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.

2 participants