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

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

Open
freyjadomville opened this issue Nov 21, 2019 · 10 comments
Labels
enhancement New feature or request module system Part of the module system migrator

Comments

@freyjadomville
Copy link

As discussed in sass/sass#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!

@freyjadomville freyjadomville changed the title Possible renaming of ~ prefixed imports Possible renaming of members inside ~ prefixed imports Nov 21, 2019
@jathak
Copy link
Member

jathak commented Nov 22, 2019

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
Copy link
Author

freyjadomville commented Nov 28, 2019 via email

@freyjadomville
Copy link
Author

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 'node_modules; 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
Copy link
Author

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.

@nex3 nex3 added the module system Part of the module system migrator label Feb 4, 2020
@l1b3r
Copy link

l1b3r commented Oct 16, 2020

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
Copy link

seyfer commented Dec 17, 2020

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
Copy link
Author

freyjadomville commented Dec 19, 2020

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
Copy link
Member

jathak commented Dec 21, 2020

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
Copy link

coltanium13 commented Nov 23, 2021

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 jathak changed the title Possible renaming of members inside ~ prefixed imports Improve error message when attempting to migrate code with unmigrated third-party dependencies May 31, 2022
@jathak
Copy link
Member

jathak commented May 31, 2022

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.

@jathak jathak added the enhancement New feature or request label May 31, 2022
@jathak jathak removed their assignment Jun 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request module system Part of the module system migrator
Projects
None yet
Development

No branches or pull requests

6 participants