Skip to content
This repository has been archived by the owner on Jul 23, 2023. It is now read-only.

Hyperclick breaks when import is inside if (Meteor.isClient) #122

Closed
softwarecreations opened this issue Jun 11, 2020 · 4 comments
Closed

Comments

@softwarecreations
Copy link

softwarecreations commented Jun 11, 2020

In a Meteor project with fourseven:scss package included

The following 2 file will demonstrate the bug

import React, { Component } from 'react';
// if (Meteor.isClient) import './my-styles.scss';

At first I thought the problem was the .scss
I added this as line 209 of parse-code.js
if (moduleName.indexOf('.scss')>-1) return console.log('Detected .scss import, exiting function');

But then I discovered that js-hyperclick craps out when it sees an import inside an if statement. The parser does not even seem to run.

If you comment out the if statement import, and do the hyperclick hover it underlines React or Component, etc.
If you uncomment the 2nd line, hyperclick breaks and then you can't hover over React or Component anymore, and can't hyperclick anything.

=================
A parseError occurs in parse-code.js Line 105ish
parseError SyntaxError: unknown: 'import' and 'export' may only appear at the top level

Atom 1.47 x86_64 on Linux
hyperclick 0.1.5
js-hyperclick 1.19.0

Meteor 1.10.1


You probably don't need Meteor to test the above.

Suggested quick and simple fix:
Just ignore any import statements that contain .scss on the same line.

@softwarecreations softwarecreations changed the title Hyperclick breaks when it sees SCSS imports Hyperclick breaks when import is inside if (Meteor.isClient) Jun 11, 2020
@AsaAyers
Copy link
Owner

The if (Meteor.isClient) import isn't valid JavaScript because import statements can't be conditional. I've never used Meteor, but it looks to me like it's using some non-standard extensions to the language.

js-hyperclick is powered by Babel@7, so if you put a Babel config in your project that can parse Meteor's custom syntax, it will pick it up and work.

@softwarecreations
Copy link
Author

softwarecreations commented Jun 11, 2020

Thanks for the reply and suggestion @AsaAyers
However if I make my own babel plugin that removes the import scss statements it would likely mean scss stops working in my project.

For now I've resolved the issue by hacking parse-code.js
I added code = code.replace(/import\s*'[^']*\.scss';/g, '0;');
At line 99.

I'd make a pull request to include the above hack, but I imagine you'd probably prefer to keep your code clean and to spec.

I can't invest more time in it right now so a hack is good enough for me for now.

I've asked for suggestions here: meteor/babel#30

@AsaAyers
Copy link
Owner

AsaAyers commented Jun 11, 2020

Can it be solved with a dynamic import?

if (Meteor.isClient) import('./my-styles.scss');

I just searched "Meteor Babel" on npm and found babel-preset-meteor. This could be another option, but personally I'd try to stick with standard JS if possible.

@softwarecreations
Copy link
Author

softwarecreations commented Jun 13, 2020

Hey @AsaAyers... interestingly my webapp (using fourseven:scss) still works if the SCSS is dynamically imported as per your example.

Wow, you've just opened up a whole new level of control for me. Because fourseven:scss has always felt a bit hacky and didn't seem to be a very active project I never thought of loading scss dynamically, but it actually works. I just tested loading it in a setTimeout for a dramatic test and it works as expected!

(fourseven-scss seems to be better maintained now than the last time I looked and Meteor got acquired by Tiny recently, so it should continue to improve)

But the weird part is using fourseven-scss (which is now called meteor-scss) with standard-minifiers bundles ALL resulting css files together. So the concept of loading via dynamic import seems to contradict that architecture.
in
https://github.com/Meteor-Community-Packages/meteor-scss
it says
"Each compiled source file produces a separate CSS file. (The standard-minifiers package merges them into one file afterwards.)"

Anyway, tweaking the syntax prevents the parser crash.

I'm going to keep my code.replace hack for now and will investigate whether dynamic SCSS imports affect the bundle size, etc before I refactor the whole project.

Thanks again!!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants