stealjs / steal-conditional

Conditional loading
https://stealjs.com/docs/steal-conditional.html
MIT License
6 stars 1 forks source link

Error with string substitution on stache files #38

Closed Sinjhin closed 7 years ago

Sinjhin commented 7 years ago

I am getting an error in the terminal (not in the browser) when using string substitution on stache files with done-serve.

{ Error: Error loading "haulhound-frontend@0.0.0#lane/list/list" at file:/Users/sinjhin/Workspace/haulhound/src/lane/list/list.js
Error loading "haulhound-frontend@0.0.0#lane/list/list" from "haulhound-frontend@0.0.0#platform/desktop.stache!done-autorender@1.0.0#src/autorender" at file:/Users/sinjhin/Workspace/haulhound/src/platform/desktop.stache
Error loading "steal-stache@3.0.5#steal-stache/*" at file:/Users/sinjhin/Workspace/haulhound/node_modules/steal-stache/steal-stache/*.js
ENOENT: no such file or directory, open '/Users/sinjhin/Workspace/haulhound/node_modules/steal-stache/steal-stache/*/index.js'
    at Error (native)
  errno: -2,
  code: 'ENOENT',
  syscall: 'open',
  path: '/Users/sinjhin/Workspace/haulhound/node_modules/steal-stache/steal-stache/*/index.js',
  statusCode: 404 }

There is no steal-stache/ folder within the steal-stache/ folder in my node_modules.

I am importing the view for a component using: import view from './#{haulhound-frontend/platform}.stache!'; Where haulhound-frontend/platform resolves to either desktop or mobile

Note that it actually works and loads the correct stache file and I get no error in the browser.

Sinjhin commented 7 years ago

Also, @phillipskevin just discovered this only happens when live-reload is running.

m-mujica commented 7 years ago

We have at least two issues here

1) Live reload through steal-tools run the loader with env set to build-development; steal-conditional does build-related stuff (like figure out possible module name variations and make them bundles on their own) based on the env value; @matthewp should we make live-reload run the load in development?

2) The substitution variations detection logic does not work correctly when the module names have extensions.... like ./{....}.stache!

matthewp commented 7 years ago

Hm, I don't think we can do that. Look at this commit: https://github.com/stealjs/steal-tools/commit/fb5af1c842f4b2b51442ae8f9730c9a3c05ce0e1 . How does the env cause problems?

m-mujica commented 7 years ago

Right here https://github.com/stealjs/steal-conditional/blob/984a3f529d15c74bf5ac9dcd76ebcc3f94ef1c0b/conditional.js#L249-L253

steal-conditional looks at the env to avoid evaluating the condition modules during the build. It does the glob-stuff to detect the variations and push them to loader.bundle.

m-mujica commented 7 years ago

@matthewp I'm having troubles to figure out how to fix the steal-conditional issue

The problem: detecting substitution variations

Given a condition like /jquery/#{browser}, the current algorithm does the following:

1) Removes the condition and trailing slash, which gives us: /jquery 2) Normalises the result of 1 to deal with relative paths, lookup schemes and stuff 3) The normalised name is added a star '' and run through locate, e.g `app@1.0.0#jquery/ 4) the resulting address from locate is turned into a glob pattern used to detect the variations in the filesystem, each match is normalised and pushed toloader.bundle`

That algorithm brakes easily with identifiers like: /folder/#{....}.stache!: the first step would result in /folder/.stache! and the next steps just make it worse.

A possible solution

1) Instead of replacing the condition with an empty string, I thought about replacing with some kind of placeholder, e.g: /folder/#{....}.stache! is transformed into /folder/foo.stache! 2) Normalise 1 3) The idea here was to replace the placeholder with a * so app@1.0.0#folder/foo.stache!stache would be turned into app@1.0.0#folder/*.stache (notice that I need to remove the plugin name) and locate that..... But the locate will add .js at the end file:/Users/foo/bar/folder/*.stache.js.

I don't like there are so many moving pieces here; what do you think? I don't think there is an easy way to get module addresses from identifiers, right?

matthewp commented 7 years ago

That algorithm brakes easily with identifiers like: /folder/#{....}.stache!: the first step would result in /folder/.stache! and the next steps just make it worse.

Why would the first step result in /folder/.stache!? Isn't the idea of step 1 to make it be /folder? Take everything off after and including the conditional part?

m-mujica commented 7 years ago

Why would the first step result in /folder/.stache!? Isn't the idea of step 1 to make it be /folder? Take everything off after and including the conditional part?

Currently we only remove the condition, #{....}, not everything after it. I think we don't want to do that anyways

1) in the /folder/#{....}.stache! example, we'll end up matching everything in /folder/* (css, markdown, etc) instead of only .stache files

2) with a conditional like /folder/#{subfolder}/index

folder
|--- chrome
|      |--- index.js
|--- ie
|      |--- index.js

Same as 1) we'll match a bunch of extra files removing everything after the condition.

matthewp commented 7 years ago

Ok makes sense. In that case I think your placeholder idea makes sense. /folder/__PLACEHOLDER__.stache! or whatever.

m-mujica commented 7 years ago

I created an example app where the bug showed up: https://github.com/m-mujica/steal-conditional-stache

In order to close this issue, we need to: