fuse-box / fuse-box-aurelia-loader

MIT License
14 stars 2 forks source link

Get the entrys path #22

Closed vegarringdal closed 7 years ago

vegarringdal commented 7 years ago

@stoffeastrom @arabsight @Thanood @nchanged

Today we added a hack fix for files ending the same, but we need to improve this part Was thinking we could use the entry path to make sure, because a module can have many files with same name in sub folders

Suggestion to solution for this:

so for this one: https://github.com/fuse-box/fuse-box-aurelia-loader/blob/master/src/fuse-box-aurelia-loader.ts#L341-L366

I was thinking this (look try/catch block)

let moduleId = Object.keys(FuseBox.packages)
    .find(name => path.startsWith(`${name}/`));

if (moduleId) {
    let resources = Object.keys(FuseBox.packages[moduleId].f);
    let resourceName = path.replace(`${moduleId}/`, '');
    let resourceEntry = resources.find(r => r.endsWith(resourceName + '.js'));

    // try and get entry path
    try {
       let subDirs = FuseBox.packages[moduleId].s.entry.split('/');
       if (subDirs.length > 1) {
           subDirs.pop();
           resourceEntry = subDirs.join('/') + '/' + resourceName + '.js';
       }
    } catch (e) {/*nothing atm*/};
    retunValue = `${moduleId}/${resourceEntry}`;
}

if (!this.fuseBoxExist(retunValue)) {
    debugPrint('error', 'findFuseBoxPath() failed to find', arguments);
}
nchanged commented 7 years ago

can’t you just use `FuseBox.import? it takes care of all extensions and indexes!

vegarringdal commented 7 years ago

no, not that I know of, because some packages resources will be under "package/dist/commonjs/resource.js", while aurelia ask for "package/resource".

unless there is somethink Ive missed?

arabsight commented 7 years ago

@vegarringdal a quick fix would be to change this line:

let resourceName = path.replace(`${moduleId}/`, '');

to this:

let resourceName = path.replace(moduleId, '');
nchanged commented 7 years ago

But if a package has an entry point, then import should happen automatically.

vegarringdal commented 7 years ago

@nchanged @arabsight Problem is when you register a global resource in aurelia, not importing the module/package https://github.com/vegarringdal/vGrid/blob/dev-rebuild/src/index.ts

The the path the loader gets is packagename/resource But when importing a commonjs module, the paths in that fusebox package have dist/commonjs, so I need to add this part, but I cant hardcode dist/commonjs since this might not always be the case.

The addPackage() feature also fixes this issue & resources that have no direct connection with entry (html, css) But I dont want to force people to use the new feature when we get it if not needed, so by checking the entry path I can correct this in my loader.

But maybe if main entry is under dist/commonjs, dist/commonjs should not be in the package

image

stoffeastrom commented 7 years ago

Also, If the plugin doesn't do a require('./file') and just

aurelia.globalResources(PLATFORM.moduleName('./file')

we need to add that dependency to fuse box. But that is maybe another plugin. Just mentioning it

vegarringdal commented 7 years ago

Also, If the plugin doesn't do a require('./file') and just aurelia.globalResources(PLATFORM.moduleName('./file') we need to add that dependency to fuse box. But that is maybe another plugin. Just mentioning it

@stoffeastrom The addPackage here will solve this: https://github.com/fuse-box/fuse-box/issues/170

Usually custom elements, css, and attributes that you register as a global resource in a plugin.

For now you can create seperate bundle/package to solve it

stoffeastrom commented 7 years ago

@vegarringdal yes that's one solution.. but I think it would help the user if a plugin could just work ootb...

vegarringdal commented 7 years ago

@vegarringdal a quick fix would be to change this line: let resourceName = path.replace(${moduleId}/, ''); to this: let resourceName = path.replace(moduleId, '');

@arabsight yes prb around 99%+, but if someone for some reason have multiple folders in a plugin and use the same name in any of them, that will fail :-) Thats why I figured using the entrys path would be the safest way to do it.

@nchanged When entry is dist/commonjs/index.js in a module, why do it add the "dist/commonjs" ? its not really needed is it ?

arabsight commented 7 years ago

it won't fail when keeping the slash even with sub directories with the same file name. but using entry is better we can simplify it by avoiding to look in the sub-resources array:

let moduleId = Object.keys(FuseBox.packages)
    .find(name => id.startsWith(`${name}/`));

if (moduleId) {
    let parentEntry = FuseBox.packages[moduleId].s.entry;
    let resourceName = id.replace(moduleId, '');
    let entry = parentEntry.replace(/\/([^\/]+)\/?$/, resourceName);
    return `${moduleId}/${entry}`;
}
vegarringdal commented 7 years ago

yes, true I was overthinking it :-)

vegarringdal commented 7 years ago

@arabsight

Thx for that code snippet