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

New JS API entrypoint #3056

Closed
4 of 5 tasks
nex3 opened this issue May 14, 2021 · 16 comments · Fixed by sass/embedded-host-node#84
Closed
4 of 5 tasks

New JS API entrypoint #3056

nex3 opened this issue May 14, 2021 · 16 comments · Fixed by sass/embedded-host-node#84
Labels
enhancement New feature or request JS API About the shared JS API

Comments

@nex3
Copy link
Contributor

nex3 commented May 14, 2021


Something @Awjin and I have discussed one on one but hasn't been captured on the issue tracker yet is the idea of creating new entrypoint functions for the JS API as part of the redesign of the custom importer and function APIs (#2509 and #2510). While we could potentially support new imports and functions in the existing render()/renderSync() entrypoints, there are a number of reasons why it might be better to have an entirely new entrypoint. (I'll tentatively call this entrypoint compile() to match the Dart Sass API.)

  • As we improve various aspects of the API, it makes it totally clear to users which aspects are new and supported (those used by compile()) and which are old and deprecated (those used by render()). Users won't be able to accidentally mix and match and then get broken when support for the deprecated versions is dropped.

  • We don't have to deal with tricky questions about how aspects of the old API interact with the new one. For example, we won't have to deal with users passing both old-style and new-style importers to the same render() call, and we won't have to figure out which source map options take precedence in which situations.

  • The names render() and renderSync() imply that the synchronous version is "less default", when in fact for Dart Sass the synchronous version is actually much more efficient. In the new API, we can call the entrypoints compile() and compileAsync() instead.

  • We can have compileAsync() return a promise rather than using the old-style error/value callback without making it a breaking change.

  • Cross-compilation state of the sort described in Provide a way to share state across multiple compilations #2745 won't work well with the old importer API, since it doesn't have a strong notion of "canonical URLs" that can be stable over time. By putting the new APIs in a new entrypoint, we'll be able to expose a version of that entrypoint on a state-preserving Compiler class without having to figure out how to handle old-style importers.

@nex3 nex3 added enhancement New feature or request JS API About the shared JS API labels May 14, 2021
@nex3 nex3 changed the title New JS entrypoint functions API New JS API entrypoint May 14, 2021
@nex3 nex3 added this to To do in JS API Overhaul via automation Aug 6, 2021
@nex3 nex3 moved this from To do to In design in JS API Overhaul Aug 6, 2021
@nex3
Copy link
Contributor Author

nex3 commented Aug 6, 2021

This proposal is now officially out for review. It will remain that way for at least a month (potentially longer depending on discussion), at which point we'll move on to implementation.

@nex3
Copy link
Contributor Author

nex3 commented Aug 6, 2021

@alexander-akait I'm particularly interested in your feedback here (and on the other proposals), as the developer of the most widely-used Sass plugin. See https://sass-lang.com/blog/request-for-comments-new-js-api for a human-friendly summary of the proposed changes.

@alexander-akait
Copy link

@nex3 looks much better than sass has now, especial logger and importer ❤️

@nex3
Copy link
Contributor Author

nex3 commented Sep 7, 2021

It's been a month and there have been no objections, so I'm going to mark the proposal as accepted and start merging it into the main spec directory.

@nex3 nex3 moved this from In design to In development in JS API Overhaul Sep 7, 2021
nex3 added a commit to sass/dart-sass that referenced this issue Oct 5, 2021
nex3 added a commit to sass/dart-sass that referenced this issue Oct 14, 2021
nex3 added a commit to sass/dart-sass that referenced this issue Oct 14, 2021
@roydukkey
Copy link

roydukkey commented Nov 4, 2021

Is there any benefit in allowing the new Functions spec to include an option for grouping them into custom module namespaces?

functions: {
    "sum($arg1, $arg2)": (arg1, arg2) => { /*...*/ }
    // Could become...
    "my-math.sum($arg1, $arg2)": (arg1, arg2) => { /*...*/ }
    // Or...
    "my-math": {
        "sum($arg1, $arg2)": (arg1, arg2) => { /*...*/ }
    }
}

I can see this as beneficial in allowing isolation, when packages might contain the same functions (potentially out of the consumer's control).

const externalPackageOne = {
    "sum($arg1, $arg2)": (arg1, arg2) => { /*...*/ }
}

const externalPackageTwo = {
    "sum($arg1, $arg2, $arg3, $arg4, $arg5)": (arg1, arg2, arg3, arg4, arg5) => { /*...*/ }
}

functions: {
    ...externalPackageOne,
    // 1. I'm not aware of the effects of the next code line
    // 2. P1 updated, with `foo()`, P2 already has `foo()`, I need to change my
    //     configuration, because I want `.foo()` from P1, but still want `.sum()` from P2
    ...externalPackageTwo
}

An additional, less important thought, is that module namespaces provided from the JS API may/must contain colons (:) which would indicate they are configured (JS) and not interpreted (Sass).

functions: {
    "my:math.sum($arg1, $arg2)": (arg1, arg2) => { /*...*/ },
    "my:math": { /*...*/ }
    // Oops. We have problems here.
    "sass:math.sum($arg1, $arg2)": (arg1, arg2) => { /*...*/ }
}
@use 'my:math';

@nex3
Copy link
Contributor Author

nex3 commented Nov 5, 2021

Supporting module-ized custom functions is an interesting question, although one I think we shouldn't block this feature on. I think a better solution than building it into the functions argument might be making it part of the importer API, since that already has well-defined behavior for locating and resolving URLs. That would also open the door for distributing function plugins on npm and having them automatically loaded by an importer.

@nex3
Copy link
Contributor Author

nex3 commented Dec 1, 2021

This is now usable in Dart Sass 1.45.0-rc.1! We'd love people's feedback on it before we launch a full stable release some time next week. @alexander-akait I'm particularly interested in any feedback you have on this from a sass-loader perspective.

@alexander-akait
Copy link

@nex3 Could you briefly describe all implemented features/api/etc? We already use logger API, migrate on compileAsync is not a problem. I see improvement for importer, can canonicalize can be async? Anyway other improvements?

@nex3
Copy link
Contributor Author

nex3 commented Dec 2, 2021

The summary in the original JS API blog post is still essentially accurate. You can also check out the API documentation, and I'm happy to answer any specific questions.

can canonicalize can be async?

Yes, as long as you're using compileAsync() or compileStringAsync(). However, I think you might be better off using a FileImporter which automatically handles a bunch of filesystem-specific details. This can also be async (although as always, running synchronously will be much faster).

@nex3
Copy link
Contributor Author

nex3 commented Dec 7, 2021

@alexander-akait Have you had a chance to play around with the new API?

@alexander-akait
Copy link

@nex3 Hello, right now no, our main problem - we still support node-sass, I don't understand why it developers still use it, but maintenance two versions will be the problem, maybe we need to drop node-sas support, also I think they should update docs and actively migrate developers to dart-sass

@nex3
Copy link
Contributor Author

nex3 commented Dec 10, 2021

I'd like to cut a stable release of the new API tomorrow. If you want to just try it locally to see if there are any egregious oversights before then, let me know, otherwise I'll go ahead and release it.

@alexander-akait
Copy link

alexander-akait commented Dec 10, 2021

@nex3 Does old API will continue work? If yes, let's go ahead

@nex3
Copy link
Contributor Author

nex3 commented Dec 10, 2021

Yep, it will continue to be supported as long as we're releasing 1.x releases.

@niksy
Copy link

niksy commented Dec 11, 2021

Yep, it will continue to be supported as long as we're releasing 1.x releases.

When do you expect to release 2.x version?

@nex3
Copy link
Contributor Author

nex3 commented Dec 12, 2021

We don't expect to release it until 2024 or so, very possibly later. There's a small chance we'll want to release it sooner in order to enable /-as-separator by default, but in that case we'll only remove those deprecated features we believe to be either in low global use or highly detrimental to the ecosystem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request JS API About the shared JS API
Projects
Development

Successfully merging a pull request may close this issue.

4 participants