fuse-box / fuse-box-aurelia-loader

MIT License
14 stars 2 forks source link

some notes #14

Closed arabsight closed 7 years ago

arabsight commented 7 years ago

Hi, @vegarringdal I tried the (css) update with your test repo and its working :+1: got some notes:

  1. fuse loader is supposed to replace loader-default you're referencing it here and in the test repo (its automatically pulled by the default bootstrapper but ideally we should create a bootstrapper too for fusebox), TextTemplateLoader should be part of this loader.
  2. public textPluginName = 'text'; should be removed, there's no text plugin here.
  3. if for some reason the developer defines the package option, then using ./ and ~/ wont work. so either force the developer not to define a package name or check for the presence of FuseBox.defaultPackageName and if it's there importing the developper's code would be like this: FuseBox.import('packageName/moduleId') instead of ./ (I don't know if defining the package name is a valid use-case but I tried it when I was starting with my loader and that's why I asked to add defaultPackageName to the public API of FuseBox).
  4. it would be great if you don't hard code '/dist/commonjs' (what if a library with sub-resources doesn't use '/dist/commonjs').

thanks for your hard work.

vegarringdal commented 7 years ago

@arabsight

thx :-)

1: The testTemplate loader is part of the loader, did you open the wrong one ? https://github.com/vegarringdal/fuse-box-aurelia-loader/blob/new_loader/src/fuse-box-aurelia-loader.ts#L15-L26

2: I need to check that one out more, copy paste from webpack one, or default, have some cleaning to do, thx

3: Unsure about this one, I have defined a package here on my datagrid Ive Im working one. It loads fine, but maybe Im missing something, much I dont know yet.

Here is the build process for the seperate package, after a few fixed by @nchanged its working like expected with main, and not executing on load. But but not being able to put this as part of a bundle and being force to use own bundle file is the ready why I created the issue with packages:[] or addPackage() here

Grid: aurelia-v-grid-plugin bundle/build

4: Yes I need to do something much better here, its not good, temp fix to just get it loading for the moment, maybe you had something in you loader ?

Other: Created this: https://github.com/fuse-box/fuse-box/issues/321

Because last updates using just raw plugin with break .ts import 'whatever.css :-)

arabsight commented 7 years ago

I totally opened the wrong file, my bad. regarding css: what is the expected behavior from this: import 'whatever.css' (do you expect it to inject using fusebox)?

vegarringdal commented 7 years ago

I totally opened the wrong file, my bad.

Been there, done that :joy:

regarding css: what is the expected behavior from this: import 'whatever.css' (do you expect it to inject using fusebox)?

Well atm we need to use raw plugin for the <require from ="whatever.css"></require> to work. But when you do import import 'whatever.css' we need it to work like the cssPlugin does it, and inject it. But when we use the cssplugin we break how aurelia work with require tag in html Not sure how we can solve that, if you have any suggestions then great :-) https://github.com/fuse-box/fuse-box/issues/321

arabsight commented 7 years ago

is that necessary? I don't think aurelia-cli nor systemjs skeleton loaders does that, do they? what if someone wants to do something with the css like:

import * as styles from 'somestylesheet.css';
doSomething(styles);

we don't expect it to inject here. IMO css best imported with require.

vegarringdal commented 7 years ago

I though that everyone did that

Webpack: https://github.com/aurelia/skeleton-navigation/blob/master/skeleton-typescript-webpack/src/main.ts#L4

I believe I did it the similar way when using jspm.. (dont rememeber) Maybe the css plugin should work like this

import 'css!yourstyle.css'; if you want it to inject the code..

@nchanged any comments?

nchanged commented 7 years ago

@vegarringdal yeah, we had it at some point. But removed it. (It leads to a mess) Could you comment on that one there:

https://github.com/fuse-box/fuse-box/issues/321

Thanood commented 7 years ago

Personally, I have prefered <require from="my.css"> but this conversation made me think. Maybe it's easier to import 'my.css', especially for custom attributes or elementts without a view..

arabsight commented 7 years ago

@Thanood isn't @viewResources the same as require for VMs, especially because you can use an object with as if you want to use scoped css.

Thanood commented 7 years ago

@arabsight I was just referring to @vegarringdal's comment about using import.. 😃

But anyway, I think yes, it is the same. How does the loader handle it? I guess like require, right?

arabsight commented 7 years ago

@Thanood we're discussing ideas :smiley: the more the better of course :wink: . the loader just loads modules, aurelia-templating and aurelia-templating-resources deals with the resources, so these two are the same:

@viewResources({ src: './app.css', as: 'scoped' })
<require from="./app.css" as="scoped"></require>

but we can't do this with import.