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

Computed data not computed in the necessary order #973

Closed
romaricpascal opened this issue Feb 27, 2020 · 8 comments
Closed

Computed data not computed in the necessary order #973

romaricpascal opened this issue Feb 27, 2020 · 8 comments

Comments

@romaricpascal
Copy link

romaricpascal commented Feb 27, 2020

Describe the bug
Hello,

I'm trying to use Eleventy for a multilingual site, using file extensions to set the locale (rather than a separate folder). I need to set 3 properties in this order:

  1. the locale read from the file path
  2. a key to match translated content
  3. the permalink derived from the key and the locale

The computation of the data happens in the wrong order and the permalink gets computed before the key, leading to undefined being set in the path.

To Reproduce
I've created a repo to reproduce : https://github.com/rhumaric/eleventy-computed-property-order-issue

If you run npx @11ty/eleventy, you'll notice the 'generating permalink' happens before the 'Setting key test'.

There's also some weird logs happening ahead of it, similar to those on #971

Expected behavior
I'd have expected Eleventy to detect that permalink depended on the key, as announced in the documentation.

Environment:

  • OS and Version: Ubuntu 18.04
  • Eleventy Version 0.11.0-beta.1
@zachleat
Copy link
Member

Howdy! This report actually resulted in a fairly large refactor of the computed data feature, The fix for this will ship with 0.11.0-beta.4 and I would really appreciate it if you retested after that was released!

Thank you!

@romaricpascal
Copy link
Author

Cool! I had been looking forward to it, thanks a lot for looking into it :D

I had a quick go on the test repository with the current master branch. Unfortunately, the order is still the same.

One thing I noticed is that if I add a data.key right before the if (data.locale) in the permalink computed data, the key gets computed before the permalink as I was hoping.

Is there a limitation of the number of computed properties another computed property can depend on?

@zachleat
Copy link
Member

zachleat commented Apr 23, 2020

Ah just to be clear and re-update here, work was done again ~3 days ago 😅 #1098 (comment)

@romaricpascal
Copy link
Author

Argh, it looks like it didn't fix it. I did another test with the latest commit on master and the behaviour remains the same 😞

I had a look inside Eleventy and I think what's breaking could be the fact that the computed data has an if whose condition is another computed data. If I understand correctly, the way Eleventy detects if a variable is used is by running the function once (

await fn(proxyData);
)? This means during that run the if (data.locale) in my computed property would evaluate to false (as it's computed) and the code never goes through the data.key inside the if block.

If it's that, I think it'll be tricky to work around. Parsing the function code with Acorn and looking at the AST came to mind, but:

  • it's probably expensive
  • it would break if the if bit happened in a function called by the computed property.

Personally, I'd be fine with declaring which other keys the data depends on if that's all that allows having computed properties derived from computed properties:

//...
  eleventyComputed: {
    permalink: ['locale', 'key',(data) => { /* ...  */}]
  } 
//...

@zachleat zachleat reopened this Apr 24, 2020
@zachleat
Copy link
Member

Alright so I think we’re going to go forward here with a documentation addition to solve edge cases like this with variable order resolution.

Here’s how your code runs from the inside:
image

It is indeed not catching both branches of those if statements. As you mentioned I don’t think there’s really an easy/fast/not-expensive way to fix this so here’s what I’m going to recommend and what I’ll end up adding to the documentation.

If you need to declare dependencies, just do it like this:

permalink(data) {
  [data.key, data.locale];
  // the rest of your code
}

This would also work:

permalink(data) {
    data.key;
    data.locale;
    // the rest of your code
}

Any access of those variables inside the callback will trigger the proper variable order

image

Does that seem acceptable to you? The other passing idea I had that I dismissed was relying on Object key order for variable resolution but that seems way more problematic with the Data Cascade.

@romaricpascal
Copy link
Author

romaricpascal commented Apr 28, 2020

Sounds like the most pragmatic solution 😄 It's only for variables used in branches that wouldn't be run through when checking the variables dependencies anyways.

Might be worth putting the debug info you showed in the screenshot under debug calls so there can be a clear troubleshooting path:

Eleventy cannot spot which computed data is used inside if blocks that rely on computed data. For example, Eleventy will miss that permalink depends on key in the following:

// _data/eleventyComputed.js
module.exports = {
	permalink: function(data) {
		if (data.locale) {
			// Eleventy will not see computed data in this block
			return `${data.locale}/{data.key}`;
		}
	}
	locale: function(data) {
		return data.page.filePathStem.split('--')[1] || data.defaultLocale;
	}
	key: function(data) {
		return data.page.filePathStem.split('--')[0];
	}
}

If it looks like one of your computed data doesn't get computed after one it depends on, you can run Eleventy with DEBUG=Eleventy:computedData to check which dependencies Eleventy has found.

DEBUG=Eleventy:computedData npx @11ty/eleventy

You can then add calls to each of the missing keys before any if statements to give a little hand to Eleventy.

module.exports = {
	permalink: function(data) {
		// This will make Eleventy pick up that `permalink`
		// depends on `key`
		data.key; 
		if (data.locale) {
			return `${data.locale}/{data.key}`;
		}
	}
	locale: function(data) {
		return data.page.filePathStem.split('--')[1] || data.defaultLocale;
	}
	key: function(data) {
		return data.page.filePathStem.split('--')[0];
	}
}

That would be a good enough troubleshooting experience for me, I think.

@zachleat
Copy link
Member

Added a section to the docs for this https://www.11ty.dev/docs/data-computed/#declaring-your-dependencies

Happy to entertain some PRs to that doc with improvements!

@romaricpascal
Copy link
Author

Nice! The doc reads pretty clear to me. Thanks for sorting it all out 🙇

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

No branches or pull requests

2 participants