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

"buildURL" gets reset to parent path of config file after calling "steal.config". #1514

Open
1 of 5 tasks
ams-tschoening opened this issue Nov 8, 2019 · 8 comments
Open
1 of 5 tasks
Labels

Comments

@ams-tschoening
Copy link

ams-tschoening commented Nov 8, 2019

How often can you reproduce it?

  • Always
  • Sometimes
  • Rarely
  • Unable
  • I didn’t try

Description:

Im using the following script tag to configure and load steal.js, which worked perfectly fine with the formerly used version 0.16.45:

<script src="/[...]/js/lib/node_modules/steal/steal.js" type="text/javascript" charset="UTF-8" data-config="/[...]/js/config/stealConfig.js" data-main="main" data-env="development" > /*<![CDATA[*/ /*]]>*/</script>

stealConfig.js contains a call to steal.config({...}) with some object containing the keys baseURL and paths. The latter defines relative paths to JS-files based on baseURL and for that to work with my slightly customized directory layout, I need to provide baseURL additionally as well. By default stealcalculates it to be /[...]/js/config/ in my setup, but I need /[...]/js/ instead.

After upgrading to the most current version 2.2.4, I recognized that at some point baseURL was /[...]/js/config/ again, so loading my files failed. After debugging things, I think I found the problem to be that baseURL really gets reset always during setting configs:

this.config({ baseURL: (root === val ? "." : root) + "/" });

https://github.com/stealjs/steal/blame/8df0de1c79df2df1ad778b0e05dfdc18d2ef814d/steal.js#L5867

That line was different in the past:

configSpecial.root.set( (root === val ? "." : root)  +"/");

https://github.com/stealjs/steal/blame/e9343ac2b52447ee11e331f2b90ffd6f1c7cfc11/steal.js#L5031

During debugging I recognized that my stealConfig.js get executed and applied first as expected, which especially means that baseURL gets set to my expected value. But afterwards things get additionally configured for my script-element:

// If a configuration was passed to startup we'll use that to overwrite
// what was loaded in stealconfig.js
// This means we call it twice, but that's ok
if (config) {
	System.config(config);
}

https://github.com/stealjs/steal/blame/8df0de1c79df2df1ad778b0e05dfdc18d2ef814d/steal.js#L6379

While that doesn't contain another baseURL, it gets reset anyway because configSetter gets called again for at least data-config and contains the line always resetting baseURL. The workaround I'm implementing now is using absolute paths in my stealConfig.js, because I'm able to calculate those in that file.

var URL_JS		= System.baseURL.replace(/config\/$/, '');
var URL_NODE	= URL_JS + 'lib/node_modules/';

Expected results:

steal.js should have used my customly provided baseURL only, like was the case in the past.

Actual results:

steal.js changed baseURL internally to some recalculated value.

Environment:

Software Version
Steal version 2.2.4
Steal-tools version 2.2.2
node -v v12.13.0
npm -v 6.12.0
Browser Opera 64.0.3417.92
Operating system Windows 10 1903 x64
@matthewp
Copy link
Member

matthewp commented Nov 8, 2019

Can you provide baseURL in the script tag? data-base-url="...". Not sure it will solve your problem or not, just my initial thought as its not usually changed after loading starts.

@ams-tschoening
Copy link
Author

I can't easily for various reasons, but I guess as well it would work that way. My point is more that I was surprised to see my explicitly provided baseURL getting overwritten, which wasn't the case in the past and I can't see documented like that.

@matthewp
Copy link
Member

matthewp commented Nov 8, 2019

If you change it back does it fix your problem? Do the tests pass?

@ams-tschoening
Copy link
Author

Change what back to what and which tests? Don't know how to run some of steal if you mean those and I never provided data-base-url="..." in the script-element. It has always been placed in stealConfig.js and yes, that still works with the old version of steal.js. As said, for the new version I have a workaround using absolute paths.

The question is if how steal behaves currently makes sense?

@matthewp
Copy link
Member

matthewp commented Nov 8, 2019

I meant changing the line that's changed, configSpecial.root.set( (root === val ? "." : root) +"/");. If you put that back in your own steal.js file does it fix your app?

@ams-tschoening
Copy link
Author

ams-tschoening commented Nov 8, 2019 via email

@matthewp
Copy link
Member

matthewp commented Nov 8, 2019

Ok, is there any chance you can create a small test where changing the baseURL in stealconfig.js causes things to mess up? That will give me something to debug.

@matthewp matthewp added the bug label Nov 8, 2019
@ams-tschoening
Copy link
Author

I've reduced my app to demonstrate the issue. The contents of the following ZIP can be extracted to your web server and afterwards simply browse index.html. There should be two alerts: The first from within stealConfig.js telling about the expected baseURL, the second one after loading everything, how that URL looks like currently. That second one is the problem, because that is actually used to load dependencies configured in stealConfig.js instead of the one explicitly given in that file as well.

At the top of stealConfig.js is the following line:

const URL_NODE	= URL_JS + 'lib/node_modules/';

Simply removing URL_JS there demonstrates that really the second baseURL is used, because afterwards jQuery can not be loaded anymore.

steal_ghi_1514 w_o deps.zip

steal_ghi_1514.zip

I'm using the package manager pnpm for dependencies, so attached two ZIPs, with and without dependencies. In theory it should be enough to run npm install -g pnpm followed by pnpm install in my lib folder.

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

No branches or pull requests

2 participants