sass / migrator

Tool for migrating stylesheets to new Sass versions
MIT License
89 stars 11 forks source link

Improve error message when attempting to migrate code with unmigrated third-party dependencies #127

Open freyjadomville opened 4 years ago

freyjadomville commented 4 years ago

As discussed in https://github.com/sass/sass/issues/2575, there appears to be an issue with sass-migrator module --verbose --dry-run --migrate-deps wanting to rename ~ - prefixed imports.

Here's a minimum example that hopefully reproduces:

vendors.scss:


@import "~bootstrap/scss/functions";
@import "~bootstrap/scss/variables";
@import "~bootstrap/scss/mixins";
@import "~bootstrap/scss/root";
@import "~bootstrap/scss/reboot";
@import "~bootstrap/scss/type";
@import "~bootstrap/scss/images";
// @import "~bootstrap/scss/code";
@import "~bootstrap/scss/grid";
@import "~bootstrap/scss/tables";
@import "~bootstrap/scss/forms";
@import "~bootstrap/scss/buttons";
// @import "~bootstrap/scss/transitions";
@import "~bootstrap/scss/dropdown";
@import "~bootstrap/scss/button-group";
@import "~bootstrap/scss/input-group";
// @import "~bootstrap/scss/custom-forms";
@import "~bootstrap/scss/nav";
@import "~bootstrap/scss/navbar";
// @import "~bootstrap/scss/card";
@import "~bootstrap/scss/breadcrumb";
// @import "~bootstrap/scss/pagination";
// @import "~bootstrap/scss/badge";
// @import "~bootstrap/scss/jumbotron";
// @import "~bootstrap/scss/alert";
// @import "~bootstrap/scss/progress";
// @import "~bootstrap/scss/media";
// @import "~bootstrap/scss/list-group";
// @import "~bootstrap/scss/close";
// @import "~bootstrap/scss/modal";
// @import "~bootstrap/scss/tooltip";
// @import "~bootstrap/scss/popover";
// @import "~bootstrap/scss/carousel";
@import "~bootstrap/scss/utilities";
// @import "~bootstrap/scss/print";

main.scss:

@import "vendors";
/*insert other imports as desired.*/

The error I receive is

C:\Users\fdomville\Git\redacted-project\>sass-migrator module --verbose --dry-run --migrate-deps scss\main.scss Error: The migrator wants to rename a member in node_modules\bootstrap\scss\_functions.scss, but it is not being migrated. You should re-run the migrator with --migrate-deps or with node_modules\bootstrap\scss\_functions.scss as one of your entrypoints. Migration failed!

jathak commented 4 years ago

Do you happen to refer to any Sass members that start with a dash or underscore and were defined by Bootstrap within your code? I'm guessing that's the most likely culprit for this error.

freyjadomville commented 4 years ago

I'll have to check in the morning, it's been a busy week and forgot to follow up to see if you had any questions.

On Fri, Nov 22, 2019 at 9:56 PM Jennifer Thakar notifications@github.com wrote:

Do you happen to refer to any Sass members that start with a dash or underscore and were defined by Bootstrap within your code? I'm guessing that's the most likely culprit for this error.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/sass/migrator/issues/127?email_source=notifications&email_token=AK6V7OXG5T6FCWQADJKWE2DQVBISRA5CNFSM4JP7UB52YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEE67R5Y#issuecomment-557709559, or unsubscribe https://github.com/notifications/unsubscribe-auth/AK6V7OSJKY5LOXFMD73H4YLQVBISRANCNFSM4JP7UB5Q .

freyjadomville commented 4 years ago

We aren't doing so directly, below is my empty search results for the solution (It's an old ASP.NET WebForms project, hence the VS screen). 'dist' and 'nodemodules; are not in the solution path and scss (seen above) is present completely - we're using webpack to run the SASS compilation in a pre-build MSBuild step. Searching for `@include ` only shows references to our own internal mixins.

image

If that is still potentially the issue, variables.scss in bootstrap does use _ prefixed functions as assertions. variables.scss is prefixed as per the minimal example. I've included a screenshot of my VSCode search results inside the node_modules folder:

image

freyjadomville commented 4 years ago

It's worth noting that I have now manually migrated the codebase from @import to @use and @forward, but I can switch things back relatively quickly if you need me to.

l1b3r commented 4 years ago

I seem to be having a similar issue with all of my node_modules imports. Here's a minimal repro though:

//theme.scss

$primary: gold;

@import "~bootstrap/scss/bootstrap";

.golden {
  color: theme-color('primary');
}
//main.scss
@import "theme";

Running sass-migrator module --migrate-deps main.scss yields

Error: The migrator wants to rename a member in node_modules\bootstrap\scss\_functions.scss, but it is not being migrated. You should re-run the migrator with --migrate-deps or with node_modules\bootstrap\scss
\_functions.scss as one of your entrypoints.
Migration failed!

Adding entire node_modules\bootstrap\scss\bootstrap.scss to the sass-migrator command line helps in resolving the minimal repro case and ends up converting entire Bootstrap's codebase in my node_modules. However, in a real project I have hundreds of those imports and I can't just go ahead and add them all into a CLI call. Also, I seem to be missing how modifying my local node_modules stylesheets would make my converted stylesheets work for other teammates.

Bootstrap 4.3.1 sass-migrator 1.2.5

seyfer commented 3 years ago

Same issue I have with ~bulma and ~buefy.

Error: Could not find Sass file at '~bulma'.
   ╷
44 │ @import "~bulma";
   │         ^^^^^^^^
   ╵
  src/assets/main.scss 44:9  root stylesheet
Migration failed!

Replaced to bulma/bulma.sass and it continued

sass-migrator --dry-run --verbose --migrate-deps --load-path node_modules module src/assets/main.scss                                                                                        15:02:47
Error: This variable was loaded from a nested import of node_modules/bulma/sass/utilities/initial-variables.sass. The module system only supports loading nested CSS using the load-css() mixin, which doesn't load variables.
   ╷
37 │ $body-background-color: $white-ter;
   │                         ^^^^^^^^^^
   ╵
  src/assets/main.scss 37:25  root stylesheet
Migration failed!

Decided to not proceed with the migration.

freyjadomville commented 3 years ago

I seem to be having a similar issue with all of my node_modules imports. Here's a minimal repro though:

//theme.scss

$primary: gold;

@import "~bootstrap/scss/bootstrap";

.golden {
  color: theme-color('primary');
}
//main.scss
@import "theme";

Running sass-migrator module --migrate-deps main.scss yields

Error: The migrator wants to rename a member in node_modules\bootstrap\scss\_functions.scss, but it is not being migrated. You should re-run the migrator with --migrate-deps or with node_modules\bootstrap\scss
\_functions.scss as one of your entrypoints.
Migration failed!

Adding entire node_modules\bootstrap\scss\bootstrap.scss to the sass-migrator command line helps in resolving the minimal repro case and ends up converting entire Bootstrap's codebase in my node_modules. However, in a real project I have hundreds of those imports and I can't just go ahead and add them all into a CLI call. Also, I seem to be missing how modifying my local node_modules stylesheets would make my converted stylesheets work for other teammates.

Bootstrap 4.3.1 sass-migrator 1.2.5

The problem with doing this task of adding to the command line is that the migrator should recognise that bootstrap is a dependency and not convert those particular imports, regardless of where they came from. Even if the solution basically means it does a re-export of the relevant code for namespacing reasons.

jathak commented 3 years ago

Yeah, the migrator's current behavior when dependencies aren't migrated is non-ideal. Our general recommendation is that users wait to migrate until all of their dependencies have migrated, as libraries may remove prefixes / implicit dependencies when migrating and rely on import-only files to avoid breaking users. Support for migrating only certain imports is something we could look into though.

coltanium13 commented 2 years ago

I have the same issue as OP. The sass migrator tool shows them using bootstrap, but I can never get it to work. Its the same exact error as OP.

C:\Users\myStuff>sass-migrator module --verbose --dry-run --migrate-deps scss\index.scss Error: The migrator wants to rename a member in node_modules\bootstrap\scss_functions.scss, but it is not being migrated. You should re-run the migrator with --migrate-deps or with node_modules\bootstrap\scss_functions.scss as one of your entrypoints. Migration failed!

This is with the latest bootstrap v5, sass package, and sass-migrator tool.

Did this ever get fixed, or is node_modules bootstrap package just not going to work with sass-migrator? what am i doing wrong?

here is most of my index.scss before the migrator tool is run


// Third-party styles
@import "~bootstrap/scss/_functions";
@import "~bootstrap/scss/_variables";
@import "bootstrap-override/_variables";
@import "~bootstrap/scss/bootstrap";

// Core
@import "~core/styles/_variables";
@import "_variables";
@import "_elements";
@import "~core/styles/_print";
@import "_utilities";
@import "_aspect-ratio";

// Shared Bootstrap Overrides
@import "~core/styles/bootstrap-override/_forms";
@import "~core/styles/bootstrap-override/_modal";
@import "~core/styles/bootstrap-override/utilities/_flex";
@import "~core/styles/bootstrap-override/utilities/_spacing";
@import "~core/styles/bootstrap-override/_card";

// Third-party overrides
@import "bootstrap-override/_badge";
@import "bootstrap-override/_buttons";
@import "bootstrap-override/_card";
@import "bootstrap-override/_dropdown";```
jathak commented 2 years ago

Sorry for the delay responding here.

Ultimately, the issue is that you're attempting to migrate code that depends on an unmigrated third-party library via multiple entrypoints, which isn't really possible (while it's possible to migrate code depending on an unmigrated library in simple cases, it's not always possible in more complex cases like this). Retargeting this issue to give a more useful error message.