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

Error with string substitution on stache files #38

Closed
Sinjhin opened this issue Apr 20, 2017 · 9 comments
Closed

Error with string substitution on stache files #38

Sinjhin opened this issue Apr 20, 2017 · 9 comments
Assignees

Comments

@Sinjhin
Copy link

Sinjhin commented Apr 20, 2017

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

Sinjhin commented Apr 20, 2017

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

@m-mujica m-mujica self-assigned this Apr 26, 2017
@m-mujica
Copy link
Contributor

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

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

@m-mujica
Copy link
Contributor

Right here

var isBuild = (loader.env || "").indexOf("build") === 0;
return isBuild ?
handleConditionalBuild() :
handleConditionalEval(m);

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

m-mujica commented Apr 26, 2017

@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 to loader.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
Copy link
Member

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

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

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

m-mujica pushed a commit that referenced this issue Apr 27, 2017
a) modules using plugins, e.g: "#{foo}.css!"
b) substitution of folder names, e.g: "/foo/#{platform}/index.js"

Closes #38
m-mujica pushed a commit that referenced this issue Apr 27, 2017
a) modules using plugins, e.g: "#{foo}.css!"
b) substitution of folder names, e.g: "/foo/#{platform}/index.js"

Closes #38
m-mujica pushed a commit that referenced this issue Apr 27, 2017
a) modules using plugins, e.g: "#{foo}.css!"
b) substitution of folder names, e.g: "/foo/#{platform}/index.js"

Closes #38
m-mujica pushed a commit that referenced this issue Apr 27, 2017
a) modules using plugins, e.g: "#{foo}.css!"
b) substitution of folder names, e.g: "/foo/#{platform}/index.js"

Closes #38
m-mujica pushed a commit to stealjs/steal-tools that referenced this issue Apr 27, 2017
1. Substitution in modules with plugins
2. Substitution of folder names

Part of stealjs/steal-conditional/issues/38
m-mujica pushed a commit that referenced this issue May 8, 2017
a) modules using plugins, e.g: "#{foo}.css!"
b) substitution of folder names, e.g: "/foo/#{platform}/index.js"

Closes #38
@m-mujica
Copy link
Contributor

m-mujica commented May 8, 2017

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:

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

No branches or pull requests

3 participants